Update the length @remote_params_len only if the related
@remote_params_val has also been set.
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Set the eventID for remoteRelayDomainQemuMonitorEvent explicitly to an
invalid value. Although the value is not used by
remoteRelayDomainQemuMonitorEvent, but it might be less prone to
errors for further refactorings.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Use stream->prog instead of a hard-coded "remoteProgram" since at
stream creation in daemonCreateClientStream "remoteProgram" is used
so we should use that especially since these functions are intended
as generic helpers for streams.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Refactor testConnectClose as it's then obvious that conn->privateData
is set to NULL in all cases. In addition, 'testConnectCloseInternal'
can be better reused.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Rename the variable @defaultConn to @defaultPrivconn as it doesn't
point to a default connection but to the private data used for the
shared default connection of the test driver.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Set privateData to NULL also for a connection that uses @defaultConn
as privateData regardless of whether @defaultConn was freed or
not. @defaultConn is shared between multiple connections and it's
ensured that there will be no memory leak by counting references.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Add typedef for the anonymous enum used for the driver features. This
allows the usage of the type in a switch statement and taking
advantage of the compilers feature to detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Add and use qemuProcessEventFree for freeing qemuProcessEvents. This
is less error-prone as the compiler can help us make sure that for
every new enumeration value of qemuProcessEventType the
qemuProcessEventFree function has to be adapted.
All process*Event functions are *only* called by
qemuProcessHandleEvent and this function does the freeing by itself
with qemuProcessEventFree. This means that an explicit freeing of
processEvent->data is no longer required in each process*Event
handler.
The effectiveness of this change is also demonstrated by the fact that
it fixes a memory leak of the panic info data in
qemuProcessHandleGuestPanic.
Reported-by: Wang Dong <dongdwdw@linux.vnet.ibm.com>
Signed-off-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Use a switch statement instead of if-else-if statements. Move the
command line building of the iothread attribute into the common path
as the SCSI controller attributes are already validated.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add a check if it's a iSCSI hostdev and if it's not then don't use the
union member 'iscsi'. The segmentation fault occured when accessing
secinfo->type, but this can vary from case to case.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
'Squash' virNetServerClientNeedAuthLocked into
virNetServerClientNeedAuth and remove virNetServerClientNeedAuthLocked
as it's not longer needed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
There is a race between virNetServerProcessClients (main thread) and
remoteDispatchAuthList/remoteDispatchAuthPolkit/remoteSASLFinish (worker
thread) that can lead to decrementing srv->nclients_unauth when it's
zero. Since virNetServerCheckLimits relies on the value
srv->nclients_unauth the underrun causes libvirtd to stop accepting
new connections forever.
Example race scenario (assuming libvirtd is using policykit and the
client is privileged):
1. The client calls the RPC remoteDispatchAuthList =>
remoteDispatchAuthList is executed on a worker thread (Thread
T1). We're assuming now the execution stops for some time before
the line 'virNetServerClientSetAuth(client, 0)'
2. The client closes the connection irregularly. This causes the
event loop to wake up and virNetServerProcessClient to be
called (on the main thread T0). During the
virNetServerProcessClients the srv lock is hold. The condition
virNetServerClientNeedAuth(client) will be checked and as the
authentication is not finished right now
virNetServerTrackCompletedAuthLocked(srv) will be called =>
--srv->nclients_unauth => 0
3. The Thread T1 continues, marks the client as authenticated, and
calls virNetServerTrackCompletedAuthLocked(srv) =>
--srv->nclients_unauth => --0 => wrap around as nclient_unauth is
unsigned
4. virNetServerCheckLimits(srv) will disable the services forever
To fix it, add an auth_pending field to the client struct so that it
is now possible to determine if the authentication process has already
been handled for this client.
Setting the authentication method to none for the client in
virNetServerProcessClients is not a proper way to indicate that the
counter has been decremented, as this would imply that the client is
authenticated.
Additionally, adjust the existing test cases for this new field.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Combine virNetServerClientSetAuth(client,
VIR_NET_SERVER_SERVICE_AUTH_NONE) and virNetServerTrackCompletedAuth
into one new function named virNetServerSetClientAuthenticated.
After using this new function the function
virNetServerTrackCompletedAuth was superfluous and is therefore
removed. In addition, it is not very common that a
'{{function}}' (virNetServerTrackCompletedAuth) does more than just
the locking compared to
'{{function}}Locked' (virNetServerTrackCompletedAuthLocked).
virNetServerTrackPendingAuth was already superfluous and therefore
it's also removed.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
The lock for @client must not only be held for the duration of
checking whether the client wants to close, but also for as long as
we're closing the client. The same applies to the tracking of
authentications.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add virNetServerClientAuthMethodImpliesAuthenticated() for deciding
whether a authentication method implies that a client is automatically
authenticated or not. Use this new function in
virNetServerClientNeedAuthLocked().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Add typedef for the anonymous enum used for the authentication methods
and remove the default case. This allows the usage of the type in a
switch statement and taking advantage of the compilers feature to
detect uncovered cases.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Direct leak of 104 byte(s) in 1 object(s) allocated from:
#0 0x7f904bfbe12b (/lib64/liblsan.so.0+0xe12b)
#1 0x7f904ba0ad67 in virAlloc ../../src/util/viralloc.c:144
#2 0x7f904bbc11a4 in virNetMessageNew ../../src/rpc/virnetmessage.c:42
#3 0x7f904bbb8e77 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:392
#4 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
#5 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
#6 0x403bed in virTestRun ../../tests/testutils.c:180
#7 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
#8 0x404c80 in virTestMain ../../tests/testutils.c:1119
#9 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
#10 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)
Indirect leak of 4 byte(s) in 1 object(s) allocated from:
#0 0x7f904bfbe12b (/lib64/liblsan.so.0+0xe12b)
#1 0x7f904ba0adc7 in virAllocN ../../src/util/viralloc.c:191
#2 0x7f904bbb8ec7 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:395
#3 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440
#4 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55
#5 0x403bed in virTestRun ../../tests/testutils.c:180
#6 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146
#7 0x404c80 in virTestMain ../../tests/testutils.c:1119
#8 0x4030d5 in main ../../tests/virnetserverclienttest.c:152
#9 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889)
SUMMARY: LeakSanitizer: 108 byte(s) leaked in 2 allocation(s).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Replace the error message during startup of libvirtd with an info
message if audit_level < 2 and audit is not supported by the
kernel. Audit is not supported by the current kernel if the kernel
does not have audit compiled in or if audit is disabled (e.g. by the
kernel cmdline).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Don't leak @blockNodes in the loop.
==226576== 7,120 bytes in 60 blocks are definitely lost in loss record 122 of 125
==226576== at 0x4835214: calloc (vg_replace_malloc.c:711)
==226576== by 0x4950D7B: virAllocN (viralloc.c:191)
==226576== by 0x49EB5BB: virXPathNodeSet (virxml.c:676)
==226576== by 0x104DB67: virQEMUCapsLoadCPUModels (qemu_capabilities.c:3738)
==226576== by 0x105510D: virQEMUCapsLoadCache (qemu_capabilities.c:3929)
==226576== by 0x104459F: qemuTestParseCapabilities (testutilsqemu.c:498)
==226576== by 0x1040DC9: testQemuCapsCopy (qemucapabilitiestest.c:105)
==226576== by 0x1041F07: virTestRun (testutils.c:180)
==226576== by 0x1040B45: mymain (qemucapabilitiestest.c:181)
==226576== by 0x104320F: virTestMain (testutils.c:1119)
==226576== by 0x1041149: main (qemucapabilitiestest.c:193)
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
This commit fixes the deadlock introduced by commit
0980764dee. The call getgrouplist() of
the glibc library isn't safe to be called in between fork and
exec (see commit 75c125641a).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Fixes: 0980764dee ("util: share code between virExec and virCommandExec")
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Since the switch statement is already using the deref'd @cap variable
and the VIR_NODE_DEV_CAP_NET case uses it, the SCSI_HOST and PCI_DEV
cases may as well use it too.
Suggested-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Use the return value of virObjectRef directly. This way, it's easier
for another reader to identify the reason why the additional reference
is required.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Implement qemuMonitorRegister() as there is already a
qemuMonitorUnregister() function. This way it may be easier to
understand the code paths.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
This way qemuDomainLogContextRef() and qemuDomainLogContextFree() is
no longer needed. The naming qemuDomainLogContextFree() was also
somewhat misleading. Additionally, it's easier to turn
qemuDomainLogContext in a self-locking object.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
There were multiple race conditions that could lead to segmentation
faults. The first precondition for this is qemuProcessLaunch must fail
sometime shortly after starting the new QEMU process. The second
precondition for the segmentation faults is that the new QEMU process
dies - or to be more precise the QEMU monitor has to be closed
irregularly. If both happens during qemuProcessStart (starting a
domain) there are race windows between the thread with the event
loop (T1) and the thread that is starting the domain (T2).
First segmentation fault scenario:
If qemuProcessLaunch fails during qemuProcessStart the code branches
to the 'stop' path where 'qemuMonitorSetDomainLog(priv->mon, NULL,
NULL, NULL)' will set the log function of the monitor to NULL (done in
T2). In the meantime the event loop of T1 will wake up with an EOF
event for the QEMU monitor because the QEMU process has died. The
crash occurs if T1 has checked 'mon->logFunc != NULL' in qemuMonitorIO
just before the logFunc was set to NULL by T2. If this situation
occurs T1 will try to call mon->logFunc which leads to the
segmentation fault.
Solution:
Require the monitor lock for setting the log function.
Backtrace:
0 0x0000000000000000 in ?? ()
1 0x000003ffe9e45316 in qemuMonitorIO (watch=<optimized out>,
fd=<optimized out>, events=<optimized out>, opaque=0x3ffe08aa860) at
../../src/qemu/qemu_monitor.c:727
2 0x000003fffda2e1a4 in virEventPollDispatchHandles (nfds=<optimized
out>, fds=0x2aa000fd980) at ../../src/util/vireventpoll.c:508
3 0x000003fffda2e398 in virEventPollRunOnce () at
../../src/util/vireventpoll.c:657
4 0x000003fffda2ca10 in virEventRunDefaultImpl () at
../../src/util/virevent.c:314
5 0x000003fffdba9366 in virNetDaemonRun (dmn=0x2aa000cc550) at
../../src/rpc/virnetdaemon.c:818
6 0x000002aa00024668 in main (argc=<optimized out>, argv=<optimized
out>) at ../../daemon/libvirtd.c:1541
Second segmentation fault scenario:
If qemuProcessLaunch fails it will unref the log context and with
invoking qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL)
qemuDomainLogContextFree() will be invoked. qemuDomainLogContextFree()
invokes virNetClientClose() to close the client and cleans everything
up (including unref of _virLogManager.client) when virNetClientClose()
returns. When T1 is now trying to report 'qemu unexpectedly closed the
monitor' libvirtd will crash because the client has already been
freed.
Solution:
As the critical section in qemuMonitorIO is protected with the monitor
lock we can use the same solution as proposed for the first
segmentation fault.
Backtrace:
0 virClassIsDerivedFrom (klass=0x3100979797979797,
parent=0x2aa000d92f0) at ../../src/util/virobject.c:169
1 0x000003fffda659e6 in virObjectIsClass (anyobj=<optimized out>,
klass=<optimized out>) at ../../src/util/virobject.c:365
2 0x000003fffda65a24 in virObjectLock (anyobj=0x3ffe08c1db0) at
../../src/util/virobject.c:317
3 0x000003fffdba4688 in
virNetClientIOEventLoop (client=client@entry=0x3ffe08c1db0,
thiscall=thiscall@entry=0x2aa000fbfa0) at
../../src/rpc/virnetclient.c:1668
4 0x000003fffdba4b4c in
virNetClientIO (client=client@entry=0x3ffe08c1db0,
thiscall=0x2aa000fbfa0) at ../../src/rpc/virnetclient.c:1944
5 0x000003fffdba4d42 in
virNetClientSendInternal (client=client@entry=0x3ffe08c1db0,
msg=msg@entry=0x2aa000cc710, expectReply=expectReply@entry=true,
nonBlock=nonBlock@entry=false) at ../../src/rpc/virnetclient.c:2116
6 0x000003fffdba6268 in
virNetClientSendWithReply (client=0x3ffe08c1db0, msg=0x2aa000cc710) at
../../src/rpc/virnetclient.c:2144
7 0x000003fffdba6e8e in virNetClientProgramCall (prog=0x3ffe08c1120,
client=<optimized out>, serial=<optimized out>, proc=<optimized out>,
noutfds=<optimized out>, outfds=0x0, ninfds=0x0, infds=0x0,
args_filter=0x3fffdb64440
<xdr_virLogManagerProtocolDomainReadLogFileArgs>, args=0x3ffffffe010,
ret_filter=0x3fffdb644c0
<xdr_virLogManagerProtocolDomainReadLogFileRet>, ret=0x3ffffffe008) at
../../src/rpc/virnetclientprogram.c:329
8 0x000003fffdb64042 in
virLogManagerDomainReadLogFile (mgr=<optimized out>, path=<optimized
out>, inode=<optimized out>, offset=<optimized out>, maxlen=<optimized
out>, flags=0) at ../../src/logging/log_manager.c:272
9 0x000003ffe9e0315c in qemuDomainLogContextRead (ctxt=0x3ffe08c2980,
msg=0x3ffffffe1c0) at ../../src/qemu/qemu_domain.c:4422
10 0x000003ffe9e280a8 in qemuProcessReadLog (logCtxt=<optimized out>,
msg=msg@entry=0x3ffffffe288) at ../../src/qemu/qemu_process.c:1800
11 0x000003ffe9e28206 in qemuProcessReportLogError (logCtxt=<optimized
out>, msgprefix=0x3ffe9ec276a "qemu unexpectedly closed the monitor")
at ../../src/qemu/qemu_process.c:1836
12 0x000003ffe9e28306 in
qemuProcessMonitorReportLogError (mon=mon@entry=0x3ffe085cf10,
msg=<optimized out>, opaque=<optimized out>) at
../../src/qemu/qemu_process.c:1856
13 0x000003ffe9e452b6 in qemuMonitorIO (watch=<optimized out>,
fd=<optimized out>, events=<optimized out>, opaque=0x3ffe085cf10) at
../../src/qemu/qemu_monitor.c:726
14 0x000003fffda2e1a4 in virEventPollDispatchHandles (nfds=<optimized
out>, fds=0x2aa000fd980) at ../../src/util/vireventpoll.c:508
15 0x000003fffda2e398 in virEventPollRunOnce () at
../../src/util/vireventpoll.c:657
16 0x000003fffda2ca10 in virEventRunDefaultImpl () at
../../src/util/virevent.c:314
17 0x000003fffdba9366 in virNetDaemonRun (dmn=0x2aa000cc550) at
../../src/rpc/virnetdaemon.c:818
18 0x000002aa00024668 in main (argc=<optimized out>, argv=<optimized
out>) at ../../daemon/libvirtd.c:1541
Other code parts where the same problem was possible to occur are
fixed as well (qemuMigrationFinish, qemuProcessStart, and
qemuDomainSaveImageStartVM).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reported-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Fix incorrect jump labels in error paths as the stop jump is only
needed if the driver has already changed the state. For example
'virAtomicIntInc(&driver->nactive)' will be 'reverted' in the
qemuProcessStop call.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The functions in virCommand() after fork() must be careful with regard
to accessing any mutexes that may have been locked by other threads in
the parent process. It is possible that another thread in the parent
process holds the lock for the virQEMUDriver while fork() is called.
This leads to a deadlock in the child process when
'virQEMUDriverGetConfig(driver)' is called and therefore the handshake
never completes between the child and the parent process. Ultimately
the virDomainObjectPtr will never be unlocked.
It gets much worse if the other thread of the parent process, that
holds the lock for the virQEMUDriver, tries to lock the already locked
virDomainObject. This leads to a completely unresponsive libvirtd.
It's possible to reproduce this case with calling 'virsh start XXX'
and 'virsh managedsave XXX' in a tight loop for multiple domains.
This commit fixes the deadlock in the same way as it is described in
commit 61b52d2e38.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
We have to allocate first and if, and only if, it was successful we
can set the count. A segfault has occurred in
virNetServerServiceNewPostExecRestart() when VIR_ALLOC_N(svc->socks,
n) has failed, but svc->nsocsk = n was already set. Thus
virObejectUnref(svc) was called and therefore it was possible that
virNetServerServiceDispose was called => segmentation fault. For
safeness NULL pointer check were added in
virNetServerServiceDispose().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
This commit removes the handcrafted code for
remoteDomainCreateWithFlags() and lets it auto generate.
A little bit of history repeating...
Commit 03d813bbcd removed the auto generation of
remoteDomainCreateWithFlags() because it was thought that the design
flaw in the remote protocol for virDomainCreate is also within the
remote protocol for virDomainCreateWithFlags. As the commit message of
ddaf15d7a3 mentions this is not the case therefore we
can auto generate the client part.
Even worse there was a typo in remoteDomainCreateWithFlags()
'remote_domain_create_with_flags_args ret;' but in fact it has to be
'remote_domain_create_with_flags_ret ret;'.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
After freeing the data structures we have to reset the counters to
zero. This fixes a segmentation fault when virNetDevIPInfoClear is
called twice (e.g. this is possible in virDomainNetDefParseXML() if
virDomainNetIPInfoParseXML(...) fails with ret < 0 (this leads to the
first call of 'virNetDevIPInfoClear(&def->guestIP)') and the resulting
call of virDomainNetDefFree(def) in the error path of
virDomainNetDefParseXML() (this leads to the second call of
virNetDevIPInfoClear(&def->guestIP), and finally to the segmentation
fault).
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
If virDomainChrSourceDefNew(xmlopt) fails, it will lead to free()ing
the uninitialized pointer bus. The fix for this is to initialize bus
with NULL.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Check if virQEMUCapsNewCopy(...) has failed, thus a segmentation fault
in virQEMUCapsFilterByMachineType(...) will be avoided.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
This patch detects a misconfiguration between the disk bus type and disk
address type for controller based disk buses (SATA, SCSI, FDC and
IDE). The addresses of these bus types are all managed in common code so
it's possible to decide in common code whether the disk address and bus
type are compatible or not.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
This function will be needed by the QEMU driver in an upcoming
patch. Additionally, removed a useless empty line.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add test cases for address conflicts between disks and hostdevs that are
using drive addresses.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Add a global check for duplicate drive addresses. This will fix the
problem of duplicate disk and hostdev drive addresses.
Example for duplicate drive addresses:
<disk>
...
<target name='sda'/>
</disk>
<disk>
...
<target name='sdb'/>
<address type='drive' controller=0 bus=0 target=0 unit=0/>
</disk>
Another example:
<hostdev mode='subsystem' type='scsi' managed='no'>
<source>
...
</source>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</hostdev>
<hostdev mode='subsystem' type='scsi' managed='no'>
<source>
...
</source>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</hostdev>
Unfortunately the fixes (1b08cc170a,
8d46386bfe) weren't enough to catch these
cases and it isn't possible to add additional checks in
virDomainDeviceDefPostParseInternal() for SCSI hostdevs or
virDomainDiskDefAssignAddress() for SCSI/IDE/FDC/SATA disks without
adding another parse flag (virDomainDefParseFlags) to disable this
validation while updating or detaching a disk or hostdev.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Don't use duplicate disk addresses in test cases unless it's useful. At
least the test case will break once we have a check for uniqueness of
addresses at time of domain definition.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Comparing the parameter 'type' against the member 'bus' instead of
against the member 'type' is quite confusing. Rename the parameter
'type' to 'bus_type' to clarify its meaning.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Pass the virDomainDeviceDriveAddress as a struct instead of individual
arguments. Reworked the function descriptions.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Removed the comment 'Set the migration source' as it isn't valid anymore
and 'start it up' isn't useful as qemuProcessStart() is already a
speaking name.
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Add missing checks if a hostdev is a subsystem/SCSI device before access
the union member 'subsys'/'scsi'. Also fix indentation and simplify
qemuDomainObjCheckHostdevTaint().
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Commit b3e4401dc6 introduced a check to ignore an error if the guest
is already terminated. However the check accidentally compared
error.code with VIR_ERR_ERROR, which is an error level, not an error
code. Because of this, almost every error got silently ignored.
Fixes: b3e4401dc6 ("systemd: don't report an error if the guest is
already terminated")
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Before the variable 'bits' was initialized with 0 (commit
3470cd860d), the following bug was
possible.
A function call with an empty bitmap leads to undefined
behavior. Because if 'bitmap->map_len == 0' 'unusedBits' will be <= 0
and 'sz == 1'. So the non global and non static variable 'bits' would
have never been set. Consequently the check 'bits == 0' results in
undefined behavior.
This patch clarifies the current version of the function by handling the
empty bitmap explicitly. Also, for an empty bitmap there is obviously no
bit set so we can just return -1 (indicating no bit set) right away. The
explicit check for 'bits == 0' after the loop is unnecessary because we
only get to this point if no set bit was found.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Verify that SCSI controllers get created automatically when a SCSI disk
is hot-plugged to a domain that doesn't have a matching SCSI controller
defined already.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
The commit "qemu: hot-plug: Assume support for -device in
qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI
controller creation used in SCSI disk hot-plugging. If we are
hot-plugging a SCSI disk to a domain and there is no proper SCSI
controller defined, it results in an "error: internal error: Could not
find scsi controller with index X required for device" error.
For that reason reverting a hunk of the commit
d4d32005d6.
This patch also adds an extra comment to the code to clarify the
loop.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Marc Hartmayer