June 2018 - Devel - libvirt List Archives

[libvirt] [PATCHv2] docs: schema: Add missing <interleave> element to input device
by Lily Zhu 14 Jun '18

14 Jun '18

For the 2 optional sub-elements - <alias> and <address> of input device, make the order of them interchangeable in the XML. Signed-off-by: Lily Zhu <lizhu(a)redhat.com> --- docs/schemas/domaincommon.rng | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1d06a5e..7c7f956 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4309,12 +4309,14 @@ </element> </group> </choice> - <optional> - <ref name="alias"/> - </optional> - <optional> - <ref name="address"/> - </optional> + <interleave> + <optional> + <ref name="alias"/> + </optional> + <optional> + <ref name="address"/> + </optional> + </interleave> </element> </define> <define name="hub"> -- 1.8.3.1

2 1

[libvirt] Call to virDomainIsActive hangs forever
by Mathieu Tarral 14 Jun '18

14 Jun '18

Hi ! I'm submitting my messages on this mailing list to request a bit of help on a case that I have where a Python application makes a call to virDomainIsActive, and the call never returns. I have tried to investigate, but as there are no debug symbols for libvirt on Debian Stretch, i can only have the following GDB backtrace: (gdb) bt #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185 #1 0x00007f49026f5b76 in virCondWait () from /usr/lib/libvirt.so.0 #2 0x00007f4902808bab in ?? () from /usr/lib/libvirt.so.0 #3 0x00007f490280a433 in virNetClientSendWithReply () from /usr/lib/libvirt.so.0 #4 0x00007f490280abe2 in virNetClientProgramCall () from /usr/lib/libvirt.so.0 #5 0x00007f49027e0ea4 in ?? () from /usr/lib/libvirt.so.0 #6 0x00007f49027ea1bb in ?? () from /usr/lib/libvirt.so.0 #7 0x00007f49027b0ef3 in virDomainIsActive () from /usr/lib/libvirt.so.0 #8 0x00007f4902b7fbd0 in libvirt_virDomainIsActive () from /usr/lib/python3/dist-packages/libvirtmod.cpython-35m-x86_64-linux-gnu.so #9 0x0000558eeec696df in PyCFunction_Call () at ../Objects/methodobject.c:109 The libvirt driver used is QEMU, and i have a specific monitoring in place using virtual machine introspection: https://github.com/KVM-VMI/kvm-vmi Now this specific monitoring somehow triggers this bug, and at this point, i don't know if it's a corner case in the libvirt QEMU driver or not. That's why i would like to have your lights on this. libvirt version: 3.0.0-4 -> Could you tell me where i should look in the code ? -> Do you have more information about this virCondWait ? Which condition is it waiting for ? -> How can i get the symbols without having the recompile libvirt and install it system wide, erasing the binaries installed by the package ? Best regards, -- Mathieu Tarral

5 11

[libvirt] [dbus PATCH 00/15] StorageVolume APIs
by Katerina Koukiou 14 Jun '18

14 Jun '18

StorageVols logically should be nested under StoragePools, however this implementation does not nest the StorageVol interface under StorageVols. Reason is that the current gdbus.c implementation would not allow nesting of objects. Phrdina checked the posibility of rewritting the gdbus.c to support such requirements, but the effort stopped, since such implentation would interfere with the current lazy loading of objects. Thus, this solution appears to be the only viable. Katerina Koukiou (15): Introduce StorageVol Interface Implement ListStorageVolumes for StoragePool Interface Implement StorageVolCreateXML method for StoragePool Interface Implement Name property for StorageVol Interface Implement Key property for StorageVol Interface Implement Path property for StorageVol Interface Implement GetXMLDesc method for StorageVol Interface Implement StorageVolLookupByKey method for Connect Interface Implement StorageVolLookupByName method for StoragePool Interface Implement StorageVolLookupByPath method for Connect Interface Implement Resize method for StorageVol Interface Implement Wipe method for StorageVol Interface Implement GetInfo method for StorageVol Interface Implement Delete method for StorageVol Interface Implement StorageVolCreateXMLFrom method for StoragePool Interface data/Makefile.am | 3 +- data/org.libvirt.Connect.xml | 12 ++ data/org.libvirt.StoragePool.xml | 28 ++++ data/org.libvirt.StorageVol.xml | 51 +++++++ src/Makefile.am | 3 +- src/connect.c | 65 ++++++++ src/connect.h | 1 + src/storagepool.c | 153 +++++++++++++++++++ src/storagevol.c | 321 +++++++++++++++++++++++++++++++++++++++ src/storagevol.h | 9 ++ src/util.c | 35 +++++ src/util.h | 16 ++ tests/libvirttest.py | 17 +++ tests/test_connect.py | 15 ++ tests/test_storage.py | 57 +++++++ 15 files changed, 784 insertions(+), 2 deletions(-) create mode 100644 data/org.libvirt.StorageVol.xml create mode 100644 src/storagevol.c create mode 100644 src/storagevol.h -- 2.15.0

3 35

Re: [libvirt] [Qemu-devel] [PATCH v6 2/2] vl: fix use of --daemonize with --preconfig
by Eduardo Habkost 14 Jun '18

14 Jun '18

CCing libvir-list. On Mon, Jun 11, 2018 at 11:29:24PM +0200, Igor Mammedov wrote: > On Mon, 11 Jun 2018 16:06:07 -0300 > Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > > On Mon, Jun 11, 2018 at 03:16:25PM +0200, Igor Mammedov wrote: > > > On Fri, 8 Jun 2018 10:21:05 -0300 > > > Eduardo Habkost <ehabkost(a)redhat.com> wrote: > > > > > > > On Thu, Jun 07, 2018 at 02:00:09PM +0200, Igor Mammedov wrote: > > > > > When using --daemonize, the initial lead process will fork a child and > > > > > then wait to be notified that setup is complete via a pipe, before it > > > > > exits. When using --preconfig there is an extra call to main_loop() > > > > > before the notification is done from os_setup_post(). Thus the parent > > > > > process won't exit until the mgmt application connects to the monitor > > > > > and tells QEMU to leave the RUN_STATE_PRECONFIG. The mgmt application > > > > > won't connect to the monitor until daemonizing has completed though. > > > > > > > > > > This is a chicken and egg problem, leading to deadlock at startup. > > > > > > > > > > The only viable way to fix this is to call os_setup_post() before > > > > > the early main_loop() call when --preconfig is used. This has the > > > > > downside that any errors from this point onwards won't be handled > > > > > well by the mgmt application, because it will think QEMU has started > > > > > successfully, so not be expecting an abrupt exit. Moving as much user > > > > > input validation as possible to before the main_loop() call might help, > > > > > but mgmt application should stop assuming that QEMU has started > > > > > successfuly and use other means to collect errors from QEMU (logfile). > > > > > > > > > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> > > > > > Signed-off-by: Igor Mammedov <imammedo(a)redhat.com> > > > > > --- > > > > > v5: > > > > > * use original Daniel's patch [1], but addapt it to apply on top of > > > > > "[PATCH v3 1/2] cli: Don't run early event loop if no --preconfig was specified" > > > > > with extra comment and massage commit message a little bit. > > > > > v6: > > > > > * hide os_setup_post_done flag inside of os_setup_post() as it was in v4 > > > > > > > > > > CC: berrange(a)redhat.com > > > > > CC: mreitz(a)redhat.com > > > > > CC: pbonzini(a)redhat.com > > > > > CC: ehabkost(a)redhat.com > > > > > CC: ldoktor(a)redhat.com > > > > > CC: eblake(a)redhat.com > > > > > --- > > > > > os-posix.c | 6 ++++++ > > > > > vl.c | 6 ++++++ > > > > > 2 files changed, 12 insertions(+) > > > > > > > > > > diff --git a/os-posix.c b/os-posix.c > > > > > index 9ce6f74..0246195 100644 > > > > > --- a/os-posix.c > > > > > +++ b/os-posix.c > > > > > @@ -309,8 +309,14 @@ void os_daemonize(void) > > > > > > > > > > void os_setup_post(void) > > > > > { > > > > > + static bool os_setup_post_done; > > > > > int fd = 0; > > > > > > > > > > + if (os_setup_post_done) { > > > > > + return; > > > > > + } > > > > > + os_setup_post_done = true; > > > > > + > > > > > if (daemonize) { > > > > > if (chdir("/")) { > > > > > error_report("not able to chdir to /: %s", strerror(errno)); > > > > > diff --git a/vl.c b/vl.c > > > > > index fa44138..457ff2a 100644 > > > > > --- a/vl.c > > > > > +++ b/vl.c > > > > > @@ -4578,6 +4578,12 @@ int main(int argc, char **argv, char **envp) > > > > > parse_numa_opts(current_machine); > > > > > > > > > > /* do monitor/qmp handling at preconfig state if requested */ > > > > > + if (!preconfig_exit_requested && is_daemonized()) { > > > > > + /* signal parent QEMU to exit, libvirt treats it as a sign > > > > > + * that monitor socket is ready to accept connections > > > > > + */ > > > > > + os_setup_post(); > > > > > + } > > > > > > > > I was looking at the daemonize logic, and noticed it we have a > > > > huge amount of code between this line and the next > > > > os_setup_post() call that could either: > > > > > > > > * call exit() and/or error_report(); or > > > logging would work to the extent mentioned in commit message, > > > i.e. it' would work fine when log file is used otherwise it > > > errors will go to /dev/null > > > > > > so it should be more or less fine on this point > > > > My worry is that most users of error_report() involve an exit() > > call too. > > > > Once we have an active monitor, we must never call exit() > > directly. Even qmp_quit() doesn't call exit() directly. > Is there any reason why exit() can't be called? QMP clients don't expect the QMP socket to be closed except when using the 'quit' command. > > > > > * be unable to finish machine initialization because of > > > > chdir("/"), change_root(), or change_process_uid(). > > > this one really no go. > > > I see 2 options here, > > > > > > * move init code that opens files to early stage (before preconfig monitor) > > > or split it to open files early. > > > (I've spotted several obvious places fwcfg/vnc/replay_start/migration) > > > but there might be code somewhere in callbacks that would do it too, > > > so it rather risky to go this route. > > > (I'd do this anyways one place at the time using sanitizing > > > initialization sequence pretext.) > > > > We might have QMP commands that take file paths as input, so is > > this really an option? > I'd think that in future we would want to enable object_add in preconfig > to create backends at runtime, so yes we can't do chroot at this point > > > > > * split out signaling part that tells parent process to exit into > > > separate helper that's called once before/from main_loop(). > > > This option seems low risk and additionally error output to > > > stderr will work as it does currently (until os_setup_post()) > > > > My assumption is that separating the chdir()/stdout/stderr logic > > from the fork/daemonize/exit steps wouldn't be possible without > > breaking expectations about -daemonize. > it's already separated and that's what creates one side of problem. Is it? Right now '$QEMU -daemonize' will never call exit(0) before the child process it spawned did the chdir()/stdout/stderr/etc trick. > What I suggest is to leave it as is and move out only > len = write(daemon_pipe, &status, 1); > part of os_setup_post() to sync with parent process. That shouldn't > affect daemonizing flow on QEMU side and would let libvirt reuse parent's > exit as sync point to detect moment when monitor is available. > (patch is in testing, I'll post it tomorrow if it doesn't break tests) This will affect the daemonizing flow, won't it? It will make QEMU exit(0) before the child does the chdir()/stderr/stdout/etc cleanup. A well-behaved daemon shouldn't do this. This is probably not a problem for libvirt (which only uses -daemonize as a sync point for QMP), but possibly a problem for other users of -daemonize. > > In worst case if we can't do the later in QEMU, mgmt would have to cope with > monitor in preconfig mode without relying on parent exit(0) sync point. > (a typical daemon would fork/chroot and co in one place and clients would use > other means to detect socket availability other than watching parent process > exiting) Do we really need to make -daemonize and -preconfig work together? libvirt uses -daemonize only for its initial capability probing, which shouldn't require -preconfig at all. Even on that case, I wonder why libvirt doesn't simply create a server socket and waits for QEMU to connect instead of using -daemonize as a sync point. -- Eduardo

5 10

[libvirt] Cannot find suitable emulator for x86_64
by Mathieu Tarral 14 Jun '18

14 Jun '18

Hi ! I'm trying to reproduce a bug that i'm facing with libvirt 3.0.0 on Debian stretch. I compiled the latest stable libvirt (4.4.0), and installed it: ./autogen.sh ./configure --prefix=$HOME/usr --localstatedir=/var make sudo make install But when i try to run ~/usr/bin/virsh version, it complains with this message: virsh # version Compiled against library: libvirt 4.4.0 Using library: libvirt 4.4.0 Using API: QEMU 4.4.0 error: failed to get the hypervisor version error: internal error: Cannot find suitable emulator for x86_64 This is what i can find in the output of ~/usr/sbin/libvirtd: 2018-06-13 11:30:17.361+0000: 27502: error : virCapabilitiesDomainDataLookupInternal:736 : invalid argument: could not find capabilities for arch=x86_64 domaintype=qemu 2018-06-13 11:30:17.361+0000: 27502: error : virQEMUCapsGetDefaultVersion:1455 : internal error: Cannot find suitable emulator for x86_64 My environment has not changed, QEMU is still installed. Can you help me understand what is wrong here ? Thank you in advance ! -- Mathieu Tarral

2 10

[libvirt] [PATCH] qemuxml2argvtest: Don't leak TPM unix path
by Michal Privoznik 14 Jun '18

14 Jun '18

When testing a domain XML with TPM we overwrite UNIX socket path to mimic what qemuTPMEmulatorPrepareHost() is doing (because *PrepareHost() functions are not called from the test). But we are not doing it fully - we need to set the chardev's type too so that virDomainTPMDefFree() can free the path. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/qemuxml2argvtest.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f630185de1..daa8d4ceeb 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -551,9 +551,11 @@ testCompareXMLToArgv(const void *data) if (vm->def->tpm) { switch (vm->def->tpm->type) { case VIR_DOMAIN_TPM_TYPE_EMULATOR: + VIR_FREE(vm->def->tpm->data.emulator.source.data.file.path); if (VIR_STRDUP(vm->def->tpm->data.emulator.source.data.file.path, "/dev/test") < 0) goto cleanup; + vm->def->tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX; break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: case VIR_DOMAIN_TPM_TYPE_LAST: -- 2.16.4

2 3

[libvirt] [PATCH] docs: fix some broken HTML links
by Katerina Koukiou 14 Jun '18

14 Jun '18

Replace name with id attribute in headers to fix the links. Signed-off-by: Katerina Koukiou <kkoukiou(a)redhat.com> --- docs/api_extension.html.in | 12 ++++++------ docs/dbus.html.in | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/api_extension.html.in b/docs/api_extension.html.in index 9beec07602..41dd3573f5 100644 --- a/docs/api_extension.html.in +++ b/docs/api_extension.html.in @@ -103,7 +103,7 @@ <p>With that said, let's begin.</p> - <h2><a name='publicapi'>Defining the public API</a></h2> + <h2><a id='publicapi'>Defining the public API</a></h2> <p>The first task is to define the public API. If the new API involves an XML extension, you have to enhance the RelaxNG @@ -137,7 +137,7 @@ and <a href="api_extension/0002-add-new-public-API.patch">0002-add-new-public-API.patch</a> for example code.</p> - <h2><a name='internalapi'>Defining the internal API</a></h2> + <h2><a id='internalapi'>Defining the internal API</a></h2> <p> Each public API call is associated with a driver, such as a host @@ -166,7 +166,7 @@ <p class="example">See <a href="api_extension/0003-define-internal-driver-API.patch">0003-define-internal-driver-API.patch</a></p> - <h2><a name='implpublic'>Implementing the public API</a></h2> + <h2><a id='implpublic'>Implementing the public API</a></h2> <p> Implementing the public API is largely a formality in which we wire up @@ -201,7 +201,7 @@ <p class="example">See <a href="api_extension/0004-implement-the-public-APIs.patch">0004-implement-the-public-APIs.patch</a></p> - <h2><a name='remoteproto'>Implementing the remote protocol</a></h2> + <h2><a id='remoteproto'>Implementing the remote protocol</a></h2> <p> Implementing the remote protocol is essentially a @@ -213,7 +213,7 @@ <p class="example">See <a href="api_extension/0005-implement-the-remote-protocol.patch">0005-implement-the-remote-protocol.patch</a></p> - <h3><a name='wireproto'>Defining the wire protocol format</a></h3> + <h3><a id='wireproto'>Defining the wire protocol format</a></h3> <p> Defining the wire protocol involves making additions to: @@ -252,7 +252,7 @@ src/remote/remote_protocol.h </code></p> - <h3><a name='rpcclient'>Implement the RPC client</a></h3> + <h3><a id='rpcclient'>Implement the RPC client</a></h3> <p> Implementing the uses the rpcgen generated .h files. The remote diff --git a/docs/dbus.html.in b/docs/dbus.html.in index 6ff71d2770..4dfe4ca427 100644 --- a/docs/dbus.html.in +++ b/docs/dbus.html.in @@ -6,14 +6,14 @@ <ul id="toc"></ul> - <h2><a name="description">Description</a></h2> + <h2><a id="description">Description</a></h2> <p> libvirt-dbus wraps libvirt API to provide a high-level object-oriented API better suited for dbus-based applications. </p> - <h2><a name="git">GIT source repository</a></h2> + <h2><a id="git">GIT source repository</a></h2> <p> The D-Bus bindings source code is maintained in a <a href="https://git-scm.com/">git</a> repository available on @@ -32,7 +32,7 @@ git clone https://libvirt.org/git/libvirt-dbus.git <a href="https://libvirt.org/git/?p=libvirt-dbus.git">https://libvirt.org/git/?p=libvirt-dbus.git</a> </pre> - <h2><a name="usage">Usage</a></h2> + <h2><a id="usage">Usage</a></h2> <p> libvirt-dbus exports libvirt API using D-Bus objects with methods and -- 2.15.0

2 1

[libvirt] [PATCH] docs: schema: Add missing <interleave> element to input device
by Lily Zhu 14 Jun '18

14 Jun '18

Input device has 2 optional sub-elements - <alias> and <address> the order of which should be interchangeable in the XML. Signed-off-by: Lily Zhu <lizhu(a)redhat.com> --- docs/schemas/domaincommon.rng | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1d06a5e..999b863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4309,12 +4309,14 @@ </element> </group> </choice> + <interleave> <optional> <ref name="alias"/> </optional> <optional> <ref name="address"/> </optional> + </interleave> </element> </define> <define name="hub"> -- 1.8.3.1

2 1

[libvirt] [PATCH] qemu: ignore query-sev-capabilities errors
by Ján Tomko 14 Jun '18

14 Jun '18

The query-sev-capabilities command fails if SEV is not compiled in, even though both the command and -object sev-guest are present in that case :/ Ignore the errors to avoid spamming the logs: internal error: unable to execute QEMU command 'query-sev-capabilities': SEV feature is not available Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/qemu/qemu_monitor_json.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c5480a2d0e..aa89ea7056 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6423,6 +6423,12 @@ qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; + /* Both -object sev-guest and query-sev-capabilities can be present + * even if SEV is not available */ + if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; + } if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; -- 2.16.1

4 3

[libvirt] [PATCH] virtportallocator: Change number of ports to 65536
by Pavel Hrdina 14 Jun '18

14 Jun '18

USHRT_MAX is not good enough because the value is 65535 which specifies the number of bits in bitmap. The allowed port range is 0-65535 so we need to increase the number. We could have USHRT_MAX + 1 but let's define the number explicitly. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1590214 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/util/virportallocator.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 004a2dab81..db95a601c7 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -35,6 +35,8 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define VIR_PORT_ALLOCATOR_NUM_PORTS 65536 + typedef struct _virPortAllocator virPortAllocator; typedef virPortAllocator *virPortAllocatorPtr; struct _virPortAllocator { @@ -68,7 +70,7 @@ virPortAllocatorNew(void) if (!(pa = virObjectLockableNew(virPortAllocatorClass))) return NULL; - if (!(pa->bitmap = virBitmapNew(USHRT_MAX))) + if (!(pa->bitmap = virBitmapNew(VIR_PORT_ALLOCATOR_NUM_PORTS))) goto error; return pa; -- 2.17.1

2 1