- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: blockjob: Adjust ATTRIBUTE_NONNULL statements for qemuBlockJobDiskNew
by Peter Krempa 19 Jul '19

19 Jul '19

Commit 5ff46aaa7f added a new parameter but neglected to fix the NONNULL declarations. Reported-by: Julio Faracco <jcfaracco(a)gmail.com> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_blockjob.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Pushed under the trivial rule. diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index d07ab75c8b..b0d17963fd 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -106,7 +106,7 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, virDomainDiskDefPtr disk, qemuBlockJobType type, const char *jobname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); qemuBlockJobDataPtr qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) -- 2.21.0

1 0

[libvirt] [PATCH 00/25] qemu: Properly track blockjobs (blockdev-add saga)
by Peter Krempa 19 Jul '19

19 Jul '19

This series requires few patches I've posted, namely: https://www.redhat.com/archives/libvir-list/2019-July/msg00669.html https://www.redhat.com/archives/libvir-list/2019-June/msg01133.html which were not yet pushed upstream. For convenience you can fetch the patches including deps by: git fetch https://github.com/pipo/libvirt.git job-tracking-send Further patches will build on top of this to add metadata for individual jobs for supporting blockdev and will be posted later as they require some cleanups. Peter Krempa (25): qemu: domain: Repurpose and export helper for saving domain status XML qemu: domain: Add helper for saving config XML qemu: blockjob: Use VIR_AUTOUNREF in qemuBlockJobDataNew qemu: blockjob: Separate and unify block job (un)registration qemu: domain: Add global table of blockjobs qemu: blockjob: Register new and running blockjobs in the global table qemu: blockjob: Add string convertors for blockjob type and state enums qemu: blockjob: Export functions for allocating and registering job data qemu: blockjob: Add flag for invalid block job data qemu: domain: Store blockjob data in the status XML qemu: blockjob: Save status XML when modifying job state qemu: driver: Remove unnecessary saving of status XML tests: qemustatusxml2xml: Add test case for block job tracking qemu: blockjob: Add 'concluded' state for a block job qemu: process: Don't trigger BLOCK_JOB* events with -blockdev qemu: blockjob: Add helper to convert monitor job status to internal state qemu: Add handler for job state change event qemu: blockjob: Add modern block job event handler qemu: process: Refresh -blockdev based blockjobs on reconnect to qemu conf: export virDomainDiskBackingStoreParse conf: export virDomainDiskBackingStoreFormat qemu: blockjob: Track orphaned backing chains in blockjob status XML qemu: Detect managed persistent reservations in block job orphan chains qemu: blockjob: Unplug inherited storage chains when concluding blockjob qemu: hotplug: Transfer ownership of backing chain to block job on disk unplug src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 13 + src/libvirt_private.syms | 2 + src/qemu/qemu_blockjob.c | 465 ++++++++++++++++-- src/qemu/qemu_blockjob.h | 40 +- src/qemu/qemu_domain.c | 327 +++++++++++- src/qemu/qemu_domain.h | 9 + src/qemu/qemu_driver.c | 64 +-- src/qemu/qemu_hotplug.c | 19 +- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_process.c | 89 +++- .../blockjob-blockdev-in.xml | 403 +++++++++++++++ .../blockjob-blockdev-out.xml | 1 + tests/qemuxml2xmltest.c | 2 + 14 files changed, 1349 insertions(+), 95 deletions(-) create mode 100644 tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml create mode 120000 tests/qemustatusxml2xmldata/blockjob-blockdev-out.xml -- 2.21.0

3 54

[libvirt] [PATCH] remote: increase daemon shutdown timer to 2 minutes
by Daniel P. Berrangé 19 Jul '19

19 Jul '19

Shutting down the daemon after 30 seconds of being idle is a little bit too aggressive. Especially when using 'virsh' in single-shot mode, as opposed to interactive shell mode, it would not be unusual to have more than 30 seconds between commands. This will lead to the daemon shutting down and starting up between a series of commands. Increasing the shutdown timer to 2 minutes will make it less likely that the daemon will shutdown while the user is in the middle of a series of commands. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/remote/libvirtd.service.in | 2 +- src/rpc/virnetsocket.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 2e51429e7a..3ddf0e229b 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -25,7 +25,7 @@ EnvironmentFile=-/etc/sysconfig/libvirtd # VMs can be performed. We don't want it to stick around if # unused though, so we set a timeout. The socket activation # then ensures it gets started again if anything needs it -ExecStart=@sbindir@/libvirtd --timeout 30 $LIBVIRTD_ARGS +ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID KillMode=process Restart=on-failure diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 34a9947eb3..3282bc0817 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -137,7 +137,7 @@ static int virNetSocketForkDaemon(const char *binary) { int ret; virCommandPtr cmd = virCommandNewArgList(binary, - "--timeout=30", + "--timeout=120", NULL); virCommandAddEnvPassCommon(cmd); -- 2.21.0

2 1

[libvirt] [RFC] handling hostdev save/load net config for non SR-IOV devices
by Daniel Henrique Barboza 19 Jul '19

19 Jul '19

Hi, I have a PoC that enables partial coldplug assignment of multifunction PCI devices with managed mode. At this moment, Libvirt can't handle this scenario - the code will detach only the hostdevs from the XML, when in fact the whole IOMMU needs to be detached. This can be verified by the fact that Libvirt handles the unmanaged scenario well, as long as the user detaches the whole IOMMU beforehand. I have played with 2 approaches. The one I am planning to contribute back is a change inside virHostdevGetPCIHostDeviceList(), that adds the extra PCI devices for detach/re-attach in case a PCI Multifunction device in managed mode is presented in the XML. Now, there's a catch. Inside both virHostdevPreparePCIDevices() and virHostdevReAttachPCIDevices() there are code to save/restore the network configuration for SR-IOV devices. These functions iterates in the hostdevs list, instead of the pcidevs list I'm changing. The final result, given that the current conditions used for SR-IOV matches the conditions for multifunction PCI devices as well, is that not all virtual functions will get their network configuration saved/restored. For example, a guest that uses 3 of 4 functions of a PCI MultiFunction card, let's say functions 0,1 and 3. The code will handle the detach of all the IOMMU, including the function 2 that isn't declared in the XML. However, since function 2 isn't a hostdev, its network config will not be restored after the VM shutdown. Now comes the question: how much effort should be spent into making the network config of all the functions be restored? Is this a blocker for the whole code to be accepted or, given it is proper documented somewhere, it can be done later on? Thanks, DHB

3 6

[libvirt] [PATCH] network: Link with libxml2
by Michal Privoznik 18 Jul '19

18 Jul '19

Since fb9f6ce6253 we are including a libxml header file in the network driver but never link with it. This hasn't caused an immediate problem because in the end the network driver links with libvirt.la. But apparently, it's causing a build issue on old Ubuntu. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed under trivial & build breaker rules. src/network/Makefile.inc.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/Makefile.inc.am b/src/network/Makefile.inc.am index 52270049d5..23cf39b6f4 100644 --- a/src/network/Makefile.inc.am +++ b/src/network/Makefile.inc.am @@ -47,7 +47,7 @@ libvirt_driver_network_impl_la_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) libvirt_driver_network_impl_la_SOURCES = $(NETWORK_DRIVER_SOURCES) -libvirt_driver_network_impl_la_LIBADD = $(DBUS_LIBS) +libvirt_driver_network_impl_la_LIBADD = $(DBUS_LIBS) $(LIBXML_LIBS) libexec_PROGRAMS += libvirt_leaseshelper libvirt_leaseshelper_SOURCES = $(NETWORK_LEASES_HELPER_SOURCES) -- 2.21.0

1 0

[libvirt] [PATCH] logging: pass binary name not logfile name when enabling logging
by Daniel P. Berrangé 18 Jul '19

18 Jul '19

Instead of having each caller pass in the desired logfile name, pass in the binary name instead. The logging code can then just derive a logfile name by appending ".log". Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/remote/remote_daemon.c | 2 +- src/util/virlog.c | 20 ++++++++++---------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index bc2fb4a7fb..7cdcd61722 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -532,7 +532,7 @@ virLockDaemonSetupLogging(virLockDaemonConfigPtr config, /* Define the default output. This is only applied if there was no setting * from either the config or the environment. */ - if (virLogSetDefaultOutput("virtlockd.log", godaemon, privileged) < 0) + if (virLogSetDefaultOutput("virtlockd", godaemon, privileged) < 0) return -1; if (virLogGetNbOutputs() == 0) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 014596b280..c8de7aa687 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -467,7 +467,7 @@ virLogDaemonSetupLogging(virLogDaemonConfigPtr config, /* Define the default output. This is only applied if there was no setting * from either the config or the environment. */ - if (virLogSetDefaultOutput("virtlogd.log", godaemon, privileged) < 0) + if (virLogSetDefaultOutput("virtlogd", godaemon, privileged) < 0) return -1; if (virLogGetNbOutputs() == 0) diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index fdc9e4333a..ffdd3b84ad 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -610,7 +610,7 @@ daemonSetupLogging(struct daemonConfig *config, /* Define the default output. This is only applied if there was no setting * from either the config or the environment. */ - if (virLogSetDefaultOutput("libvirtd.log", godaemon, privileged) < 0) + if (virLogSetDefaultOutput("libvirtd", godaemon, privileged) < 0) return -1; if (virLogGetNbOutputs() == 0) diff --git a/src/util/virlog.c b/src/util/virlog.c index 248ce19902..da433878df 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -175,7 +175,7 @@ virLogSetDefaultOutputToJournald(void) static int -virLogSetDefaultOutputToFile(const char *filename, bool privileged) +virLogSetDefaultOutputToFile(const char *binary, bool privileged) { int ret = -1; char *logdir = NULL; @@ -183,8 +183,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged) if (privileged) { if (virAsprintf(&virLogDefaultOutput, - "%d:file:%s/log/libvirt/%s", virLogDefaultPriority, - LOCALSTATEDIR, filename) < 0) + "%d:file:%s/log/libvirt/%s.log", virLogDefaultPriority, + LOCALSTATEDIR, binary) < 0) goto cleanup; } else { if (!(logdir = virGetUserCacheDirectory())) @@ -197,8 +197,8 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged) } umask(old_umask); - if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s", - virLogDefaultPriority, logdir, filename) < 0) + if (virAsprintf(&virLogDefaultOutput, "%d:file:%s/%s.log", + virLogDefaultPriority, logdir, binary) < 0) goto cleanup; } @@ -211,19 +211,19 @@ virLogSetDefaultOutputToFile(const char *filename, bool privileged) /* * virLogSetDefaultOutput: - * @filename: the file that the output should be redirected to (only needed - * when @godaemon equals true + * @binary: the binary for which logging is performed. The log file name + * will be derived from the binary name, with ".log" appended. * @godaemon: whether we're running daemonized * @privileged: whether we're running with root privileges or not (session) * * Decides on what the default output (journald, file, stderr) should be - * according to @filename, @godaemon, @privileged. This function should be run + * according to @binary, @godaemon, @privileged. This function should be run * exactly once at daemon startup, so no locks are used. * * Returns 0 on success, -1 in case of a failure. */ int -virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged) +virLogSetDefaultOutput(const char *binary, bool godaemon, bool privileged) { bool have_journald = access("/run/systemd/journal/socket", W_OK) >= 0; @@ -237,7 +237,7 @@ virLogSetDefaultOutput(const char *filename, bool godaemon, bool privileged) return virLogSetDefaultOutputToStderr(); } - return virLogSetDefaultOutputToFile(filename, privileged); + return virLogSetDefaultOutputToFile(binary, privileged); } -- 2.21.0

2 3

[libvirt] [PATCH 0/5] network: xmlns dnsmasq option passthrough
by Cole Robinson 18 Jul '19

18 Jul '19

There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options. This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented. Cole Robinson (5): conf: Add virNetworkXMLOption conf: Add network xmlopt argument conf: Add virNetworkXMLNamespace network: wire up dnsmasq option xmlns docs: formatnetwork: Document xmlns:dnsmasq docs/formatnetwork.html.in | 22 +++ docs/schemas/network.rng | 11 ++ src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 91 +++++++-- src/conf/network_conf.h | 51 ++++- src/conf/virnetworkobj.c | 50 +++-- src/conf/virnetworkobj.h | 13 +- src/esx/esx_network_driver.c | 4 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 182 ++++++++++++++++-- src/network/bridge_driver.h | 12 ++ src/network/bridge_driver_platform.h | 2 + src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 11 +- src/vbox/vbox_network.c | 4 +- tests/Makefile.am | 14 +- .../networkxml2confdata/dnsmasq-options.conf | 18 ++ tests/networkxml2confdata/dnsmasq-options.xml | 15 ++ tests/networkxml2conftest.c | 8 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmlin/dnsmasq-options.xml | 15 ++ tests/networkxml2xmlout/dnsmasq-options.xml | 17 ++ tests/networkxml2xmltest.c | 11 +- tests/networkxml2xmlupdatetest.c | 4 +- 24 files changed, 478 insertions(+), 84 deletions(-) create mode 100644 tests/networkxml2confdata/dnsmasq-options.conf create mode 100644 tests/networkxml2confdata/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlin/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlout/dnsmasq-options.xml -- 2.21.0

4 11

[libvirt] [PATCH 0/2] util: Fix memleak after (legacy) blockjob (blockdev-add saga spin-off)
by Peter Krempa 18 Jul '19

18 Jul '19

While verifying that legacy block jobs are not broken I've found a memleak. Peter Krempa (2): util: storage: Clean up label use in virStorageFileGetMetadataInternal util: storage: Don't leak metadata on repeated calls of virStorageFileGetMetadata src/util/virstoragefile.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) -- 2.21.0

3 4

[libvirt] [PATCH 0/2] tools: Introduce virshNodedevCapabilityNameCompleter
by Michal Privoznik 18 Jul '19

18 Jul '19

Yet again, couple of patches I have on my local branch for the feature I'm working on and which can go separately. Michal Prívozník (2): virsh-completer: Separate comma list construction into a function tools: Introduce virshNodedevCapabilityNameCompleter tools/virsh-completer.c | 147 ++++++++++++++++++++++++++++------------ tools/virsh-completer.h | 4 ++ tools/virsh-nodedev.c | 1 + 3 files changed, 109 insertions(+), 43 deletions(-) -- 2.21.0

4 10

[libvirt] [PATCH] util: command: Ignore bitmap errors when enumerating file descriptors to close
by Peter Krempa 18 Jul '19

18 Jul '19

virCommandMassCloseGetFDsLinux fails when running libvird on valgrind with the following message: libvirt: error : internal error: unable to set FD as open: 1024 This is because valgrind opens few file descriptors beyond the limit: 65701125 lr-x------. 1 root root 64 Jul 18 14:48 1024 -> /home/pipo/build/libvirt/gcc/src/.libs/libvirtd 65701126 lrwx------. 1 root root 64 Jul 18 14:48 1025 -> '/tmp/valgrind_proc_3849_cmdline_186612e3 (deleted)' 65701127 lrwx------. 1 root root 64 Jul 18 14:48 1026 -> '/tmp/valgrind_proc_3849_auxv_186612e3 (deleted)' 65701128 lrwx------. 1 root root 64 Jul 18 14:48 1027 -> /dev/pts/11 65701129 lr-x------. 1 root root 64 Jul 18 14:48 1028 -> 'pipe:[65689522]' 65701130 l-wx------. 1 root root 64 Jul 18 14:48 1029 -> 'pipe:[65689522]' 65701131 lr-x------. 1 root root 64 Jul 18 14:48 1030 -> /tmp/vgdb-pipe-from-vgdb-to-3849-by-root-on-angien Ignore bitmap errors in this case since we'd leak those FD's anyways in the previous scenario. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/util/vircommand.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 5141611ca4..4501c96bbf 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -446,12 +446,7 @@ virCommandMassCloseGetFDsLinux(virCommandPtr cmd ATTRIBUTE_UNUSED, goto cleanup; } - if (virBitmapSetBit(fds, fd) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to set FD as open: %d"), - fd); - goto cleanup; - } + ignore_value(virBitmapSetBit(fds, fd)); } if (rc < 0) -- 2.21.0

2 1