Re: [libvirt] libxl and non-absolute paths
On Tue, Feb 17, 2015 at 08:15:34PM -0700, Jim Fehlig wrote: Stefan Bader wrote: Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. It only sets the device_model_version (LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN vs LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL). device_model is directly VIR_STRDUP'ed from def-emulator. Not sure whether the libxl driver could query libxl for the path prefix. I asked about that too, but the Xen community preferred a build-time approach. Wei Liu added a pkgconfig module to Xen, enabling build-time detection of the paths commit babeca328413baebfdca366a5b17c06acf4295e8 Author: Wei Liu wei.l...@citrix.com Date: Fri Jan 9 14:32:18 2015 + libxl: provide xenlight.pc A pkg-config file for libxl. It also contains two variables (xenfirmwaredir and libexec_bin) so that tools that are very keen on knowing the locations of Xen binaries (say, libvirt) can use them to determine the location of the binaries. We are not yet making use of xenlight.pc in libvirt. But this work aims to improve reporting the correct paths in *capabilities*. It could also be used in the xl.cfg to domXML conversion code. Right now the most straight forward way seems to move back to a full path for the emulator. Full paths are required as per the documentation. From http://libvirt.org/formatdomain.html#elementsOSBootloader The content of the |bootloader| element provides a fully qualified path to the bootloader executable in the host OS. Same for emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? There's probably a fair bit of existing config with e.g. bootlaoderpygrub/bootloader that no longer validates. ATM, I don't have a good answer :-/. Is correct capabilities information and xl.cfg - libvirt.xml conversion, along with better error reporting enough? Should users change their non-validating domXML? From libvirt's perspective, I think full paths, discovered from capabilities, should be required. I'd like to hear Daniel's opinion. He may have considered such cases when enabling the validation. Yeah, I really think it should be using full paths for bootloader too Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch 0/4] PowerPC fixes for libvirt
On Tuesday 17 February 2015 06:33 PM, Ján Tomko wrote: On Sun, Feb 15, 2015 at 09:45:25AM +0530, Prerna Saxena wrote: This patch set addresses some miscellaneous fixes for libvirt on PowerPC. Details: Patch 1/4 : This adds 'qemu-system-ppc64' as the default emulator for ppc64 ppc64le guests. Patch 2/4 : Fixes a small error where not specifying a CPU model leads to a NULL compat specification. I have pushed the first two patches. Patch 3/4 : This introduces 'ppc64le' and newer pseries machine types to domain schema. Patch 4/4 : Forbids floppy devices in domain XML. These would be better if they included some test cases (so would 2/4, but adding a different host cpu to the test suite is not that trivial). Jan Thanks for pushing patches 1,2. I will add testcases and resend patch #3. As discussed with you, I will rework Patch #4 in line with https://www.redhat.com/archives/libvir-list/2015-February/msg00410.html Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libxl and non-absolute paths
On Mon, Feb 16, 2015 at 10:26:56AM +0100, Stefan Bader wrote: On 16.02.2015 10:18, Martin Kletzander wrote: On Fri, Feb 13, 2015 at 03:20:07PM +0100, Stefan Bader wrote: Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. Not sure whether the libxl driver could query libxl for the path prefix. Right now the most straight forward way seems to move back to a full path for the emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? Maybe stupid question here... How does the string with the prefix look like then? Is it something like bootloaderpygrub:/path/to/loader ? No, sorry I should probably have added that: in both cases there is only the binary name in the config and libxl extends things internally. So bootloaderpygrub/bootloader and loaderhvmloader/loader. Sorry for late reply. I, personally, would be OK with that, I would just make sure that other drivers (e.g. QEMU) handles that as well. -Stefan -Stefan --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng 2015-01-23 12:46:24. +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616 @@ -258,7 +258,7 @@ /choice /attribute /optional -ref name=absFilePath/ +ref name=filePath/ /element /optional optional @@ -1060,7 +1060,7 @@ optional element name=bootloader choice -ref name=absFilePath/ +ref name=filePath/ empty/ /choice /element -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list pgpcxxJkvm1ue.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure
On Mon, Feb 16, 2015 at 19:51:48 +0100, Peter Krempa wrote: Due to historical madne^Wreasons the NUMA configuration is split between /domain/cpu and /domain/numatune. Internally the data was also split into two places. We can't do anything about the external representation but we certainly can store all the definitions in one place internally. This series does that. This series no longer applies cleanly after commit commit 65c0fd9dfc712d23721e8052ce655100e230a3b3 Author: Michal Privoznik mpriv...@redhat.com Date: Thu Feb 12 17:37:46 2015 +0100 numatune_conf: Expose virDomainNumatuneNodeSpecified I can repost it if required. I will also provide a convenience branch on a public repo once I finish rebasing and a few other patches. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] utilities for supporting midonet virtualports
On Wed, Feb 18, 2015 at 4:00 AM, YAMAMOTO Takashi yamam...@valinux.co.jp wrote: + * Authors: + * Antoni Segura Puimedon t...@midokura.com missing ? Indeed! thanks :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Exit job on error path of qemuDomainSetVcpusFlags()
On 18.02.2015 18:08, Peter Krempa wrote: Commit e105dc981438bc33fa771bd67cece6234dbf6c8d moved some code but didn't adjust the jump labels so that the job would be terminated. --- Notes: I'm already wearing my pink fluffy bunny ears of shame as I've acked that patch. src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Please make sure you've pushed the patch into the the maint branch as well. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Exit job on error path of qemuDomainSetVcpusFlags()
Commit e105dc981438bc33fa771bd67cece6234dbf6c8d moved some code but didn't adjust the jump labels so that the job would be terminated. --- Notes: I'm already wearing my pink fluffy bunny ears of shame as I've acked that patch. src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1bbbe9b..8f0cf2b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4716,13 +4716,13 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)) { if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0) -goto cleanup; +goto endjob; if (!(all_nodes = virNumaGetHostNodeset())) -goto cleanup; +goto endjob; if (!(all_nodes_str = virBitmapFormat(all_nodes))) -goto cleanup; +goto endjob; if (virCgroupGetCpusetMems(cgroup_temp, mem_mask) 0 || virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) 0) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Exit job on error path of qemuDomainSetVcpusFlags()
On Wed, Feb 18, 2015 at 06:08:36PM +0100, Peter Krempa wrote: Commit e105dc981438bc33fa771bd67cece6234dbf6c8d moved some code but didn't adjust the jump labels so that the job would be terminated. --- Notes: I'm already wearing my pink fluffy bunny ears of shame as I've acked that patch. src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1bbbe9b..8f0cf2b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4716,13 +4716,13 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags VIR_DOMAIN_AFFECT_LIVE !(flags VIR_DOMAIN_VCPU_GUEST)) { if (virCgroupNewEmulator(priv-cgroup, false, cgroup_temp) 0) -goto cleanup; +goto endjob; if (!(all_nodes = virNumaGetHostNodeset())) -goto cleanup; +goto endjob; if (!(all_nodes_str = virBitmapFormat(all_nodes))) -goto cleanup; +goto endjob; if (virCgroupGetCpusetMems(cgroup_temp, mem_mask) 0 || virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) 0) -- 2.2.2 ACK, and shame on me for sending that patch. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Exit job on error path of qemuDomainSetVcpusFlags()
On Wed, Feb 18, 2015 at 18:18:16 +0100, Michal Privoznik wrote: On 18.02.2015 18:08, Peter Krempa wrote: Commit e105dc981438bc33fa771bd67cece6234dbf6c8d moved some code but didn't adjust the jump labels so that the job would be terminated. --- Notes: I'm already wearing my pink fluffy bunny ears of shame as I've acked that patch. src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Please make sure you've pushed the patch into the the maint branch as well. Pushed both to master and to v1.2.12-maint. Michal Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix AAVMF/OVMF #define names
The AAVMF and OVMF names were swapped. Reorder the one usage where it matters so behavior doesn't change. --- src/qemu/qemu_conf.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c6b083c..2cf3905 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -149,10 +149,10 @@ virQEMUDriverConfigLoaderNVRAMParse(virQEMUDriverConfigPtr cfg, } -#define VIR_QEMU_OVMF_LOADER_PATH /usr/share/AAVMF/AAVMF_CODE.fd -#define VIR_QEMU_OVMF_NVRAM_PATH /usr/share/AAVMF/AAVMF_VARS.fd -#define VIR_QEMU_AAVMF_LOADER_PATH /usr/share/OVMF/OVMF_CODE.fd -#define VIR_QEMU_AAVMF_NVRAM_PATH /usr/share/OVMF/OVMF_VARS.fd +#define VIR_QEMU_OVMF_LOADER_PATH /usr/share/OVMF/OVMF_CODE.fd +#define VIR_QEMU_OVMF_NVRAM_PATH /usr/share/OVMF/OVMF_VARS.fd +#define VIR_QEMU_AAVMF_LOADER_PATH /usr/share/AAVMF/AAVMF_CODE.fd +#define VIR_QEMU_AAVMF_NVRAM_PATH /usr/share/AAVMF/AAVMF_VARS.fd virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) { @@ -313,10 +313,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; cfg-nloader = 2; -if (VIR_STRDUP(cfg-loader[0], VIR_QEMU_OVMF_LOADER_PATH) 0 || -VIR_STRDUP(cfg-nvram[0], VIR_QEMU_OVMF_NVRAM_PATH) 0 || -VIR_STRDUP(cfg-loader[1], VIR_QEMU_AAVMF_LOADER_PATH) 0 || -VIR_STRDUP(cfg-nvram[1], VIR_QEMU_AAVMF_NVRAM_PATH) 0) +if (VIR_STRDUP(cfg-loader[0], VIR_QEMU_AAVMF_LOADER_PATH) 0 || +VIR_STRDUP(cfg-nvram[0], VIR_QEMU_AAVMF_NVRAM_PATH) 0 || +VIR_STRDUP(cfg-loader[1], VIR_QEMU_OVMF_LOADER_PATH) 0 || +VIR_STRDUP(cfg-nvram[1], VIR_QEMU_OVMF_NVRAM_PATH) 0) goto error; #endif -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 03:29:35PM +, Daniel P. Berrange wrote: On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); Why don't we just not register the reload handler until we get to the point where it is safe. eg register it only after virStateInitialize is complete, and unregister it before we call virStateCleanup. In that case we should register empty handler for SIGHUP otherwise reload could exit libvirtd during driver initialization. But you have a point that my patch still doesn't fix the issue that reload could be triggered during virStateCleanup. I'll send a v2 where at first we will set empty handler. After virStateInitialize is complete we set the proper reload handler and before virStateCleanup we set again empty handler. Thanks for review. Pavel Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 05:08:28PM +0100, Pavel Hrdina wrote: On Wed, Feb 18, 2015 at 03:29:35PM +, Daniel P. Berrange wrote: On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); Why don't we just not register the reload handler until we get to the point where it is safe. eg register it only after virStateInitialize is complete, and unregister it before we call virStateCleanup. In that case we should register empty handler for SIGHUP otherwise reload could exit libvirtd during driver initialization. But you have a point that my patch still doesn't fix the issue that reload could be triggered during virStateCleanup. Oh that's a good point. I forgot about default SIGHUP behaviour. I'll send a v2 where at first we will set empty handler. After virStateInitialize is complete we set the proper reload handler and before virStateCleanup we set again empty handler. Maybe your current approach is actually simpler than having to create a dummy sighup handler function. We'd just need to reset driversInitialized to false again before virSTateCleanup Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 04:13:15PM +, Daniel P. Berrange wrote: On Wed, Feb 18, 2015 at 05:08:28PM +0100, Pavel Hrdina wrote: On Wed, Feb 18, 2015 at 03:29:35PM +, Daniel P. Berrange wrote: On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); Why don't we just not register the reload handler until we get to the point where it is safe. eg register it only after virStateInitialize is complete, and unregister it before we call virStateCleanup. In that case we should register empty handler for SIGHUP otherwise reload could exit libvirtd during driver initialization. But you have a point that my patch still doesn't fix the issue that reload could be triggered during virStateCleanup. Oh that's a good point. I forgot about default SIGHUP behaviour. I'll send a v2 where at first we will set empty handler. After virStateInitialize is complete we set the proper reload handler and before virStateCleanup we set again empty handler. Maybe your current approach is actually simpler than having to create a dummy sighup handler function. We'd just need to reset driversInitialized to false again before virSTateCleanup Agree, that's the missing peace. I'll sent v2. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 05:08:28PM +0100, Pavel Hrdina wrote: On Wed, Feb 18, 2015 at 03:29:35PM +, Daniel P. Berrange wrote: On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); Why don't we just not register the reload handler until we get to the point where it is safe. eg register it only after virStateInitialize is complete, and unregister it before we call virStateCleanup. In that case we should register empty handler for SIGHUP otherwise reload could exit libvirtd during driver initialization. But you have a point that my patch still doesn't fix the issue that reload could be triggered during virStateCleanup. I'll send a v2 where at first we will set empty handler. After virStateInitialize is complete we set the proper reload handler and before virStateCleanup we set again empty handler. Actually that would require to handle return value of virNetServerAddSignalHandler every time we need to re-register the handler for SIGHUP. So I still think that the first approach is still better. Pavel Thanks for review. Pavel Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix memory ABI stability check issues
On Mon, Feb 16, 2015 at 20:50:41 +0100, Peter Krempa wrote: Note that this series applies only on top of the NUMA config unification series: http://www.redhat.com/archives/libvir-list/2015-February/msg00532.html Please see individual patches for explanation Peter Krempa (3): conf: ABI: Hugepage backing definition is not guest ABI conf: ABI: Memballoon setting is not guest ABI conf: numa: Check ABI stability of NUMA configuration src/conf/domain_conf.c | 31 ++- src/conf/numa_conf.c | 37 + src/conf/numa_conf.h | 3 +++ src/libvirt_private.syms | 1 + 4 files changed, 43 insertions(+), 29 deletions(-) Rebased version of this series including the depending series is available at: git fetch git://pipo.sk/pipo/libvirt.git/ memory-refactors signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/24] Move all NUMA related configuration into one structure
On Wed, Feb 18, 2015 at 10:24:10 +0100, Peter Krempa wrote: On Mon, Feb 16, 2015 at 19:51:48 +0100, Peter Krempa wrote: Due to historical madne^Wreasons the NUMA configuration is split between /domain/cpu and /domain/numatune. Internally the data was also split into two places. We can't do anything about the external representation but we certainly can store all the definitions in one place internally. This series does that. This series no longer applies cleanly after commit commit 65c0fd9dfc712d23721e8052ce655100e230a3b3 Author: Michal Privoznik mpriv...@redhat.com Date: Thu Feb 12 17:37:46 2015 +0100 numatune_conf: Expose virDomainNumatuneNodeSpecified I can repost it if required. I will also provide a convenience branch on a public repo once I finish rebasing and a few other patches. Peter The rebased version including two other series that build on top of this one are now available at: git fetch git://pipo.sk/pipo/libvirt.git/ memory-refactors signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] qemu: lxc: Clarify error message when setting current memory
Commit 60f7303c151cccdbe214b9f9ac59ecaf95cbf24b introduced the error message but it's unclear whether the persistent config or the live config tripped the message. Later the LXC driver copied the same code. Separate the message which will also clarify the code. --- src/lxc/lxc_driver.c | 21 +++-- src/qemu/qemu_driver.c | 19 ++- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3adb21d..5af0554 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -742,18 +742,19 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto cleanup; } } else { -unsigned long oldmax = 0; - -if (flags VIR_DOMAIN_AFFECT_LIVE) -oldmax = vm-def-mem.max_balloon; -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -if (!oldmax || oldmax persistentDef-mem.max_balloon) -oldmax = persistentDef-mem.max_balloon; +if (flags VIR_DOMAIN_AFFECT_LIVE +newmem vm-def-mem.max_balloon) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(max memory of the live instance can't be less + than the current memory)); +goto cleanup; } -if (newmem oldmax) { -virReportError(VIR_ERR_INVALID_ARG, - %s, _(Cannot set memory higher than max memory)); +if (flags VIR_DOMAIN_AFFECT_CONFIG +newmem persistentDef-mem.max_balloon) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(max memory of the persistent configuration can't + be less than the current memory)); goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e2abbe0..4458e02 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2326,18 +2326,19 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ -unsigned long oldmax = 0; - -if (flags VIR_DOMAIN_AFFECT_LIVE) -oldmax = vm-def-mem.max_balloon; -if (flags VIR_DOMAIN_AFFECT_CONFIG) { -if (!oldmax || oldmax persistentDef-mem.max_balloon) -oldmax = persistentDef-mem.max_balloon; +if (flags VIR_DOMAIN_AFFECT_LIVE +newmem vm-def-mem.max_balloon) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(max memory of the live instance can't be less + than the current memory)); +goto endjob; } -if (newmem oldmax) { +if (flags VIR_DOMAIN_AFFECT_CONFIG +newmem persistentDef-mem.max_balloon) { virReportError(VIR_ERR_INVALID_ARG, %s, - _(cannot set memory higher than max memory)); + _(max memory of the persistent configuration can't + be less than the current memory)); goto endjob; } -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix reporting of i/o errors by iohelper process
On 02/05/2015 11:26 AM, Jason J. Herne wrote: Hello Eric, it has been quite a while since we have looked at this :). I'm revisiting this problem and I find that it still exists. Here is an archive link of our previous discussions on this: https://www.redhat.com/archives/libvir-list/2014-July/thread.html#00809 Basically, when I do a migration and run out of disk space I do not see a meaningful error message without this patch. Early on in our previous discussions you asked me how I had gotten through the qemuDomainSaveMemory function without hitting this code: if (virFileWrapperFdClose(wrapperFd) 0) goto cleanup; If qemuMigrationToFile fails we go directly to cleanup and do not pass through the virFileWrapperFdClose call. For reference: /* Perform the migration */ if (qemuMigrationToFile(driver, vm, fd, offset, path, qemuCompressProgramName(compressed), bypassSecurityDriver, asyncJob) 0) goto cleanup; This is why my patch adds the additional call to virFileWrapperFdClose. Perhaps, if we cannot tolerate calling it twice, I can adjust the code such that we only call it once. For some odd reason, you seem to see messages that I do not (take look at my final message in the thread linked above). I was not able to see your goodbye world \n message. I would be interested to know what you see when you run my test case without my patch. If you feel like trying it just create a 50MB disk image and loop mount it: mount -o loop /usr/local/var/lib/libvirt/qemu/save.img /usr/local/var/lib/libvirt/qemu/save Then: virsh managedsave guestname And report what you saw on the command line and the libvirtd console and/or log depending on how you run the daemon. I suspect (since you saw goodbye world) that you would also see the out of disk space message ok. If I'm correct, then the question becomes... why don't I see it too? :) Thanks for your time. Polite Ping? :) -- -- Jason J. Herne (jjhe...@linux.vnet.ibm.com) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] Automaticaly fill memory element for NUMA enabled guests
Conclusion of the memory refactoring series. These patches depend on the two previous series and for convenience can be fetched at: git fetch git://pipo.sk/pipo/libvirt.git/ memory-refactors This series changes the behavior of the memory element in case the guest has NUMA enabled and fills automatically the sum of sizes of numa nodes instead of relying on the user passing correct data. Peter Krempa (7): qemu: Forbid seting maximum memory size with the API with NUMA enabled qemu: lxc: Clarify error message when setting current memory conf: Hoist validation of memory size into the post parse callback conf: Replace access to def-mem.max_balloon with accessor functions qemu: command: Add helper to align memory sizes conf: numa: Add helper to count total memory size configured in NUMA conf: Automatically use NUMA memory size in case NUMA is enabled src/conf/domain_conf.c | 101 - src/conf/domain_conf.h | 4 + src/conf/numa_conf.c | 13 +++ src/conf/numa_conf.h | 1 + src/hyperv/hyperv_driver.c | 2 +- src/libvirt_private.syms | 4 + src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_driver.c | 8 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 27 +++--- src/lxc/lxc_fuse.c | 4 +- src/lxc/lxc_native.c | 4 +- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_sdk.c | 12 +-- src/phyp/phyp_driver.c | 11 ++- src/qemu/qemu_command.c| 23 +++-- src/qemu/qemu_domain.c | 21 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 44 + src/qemu/qemu_hotplug.c| 8 +- src/qemu/qemu_process.c| 2 +- src/test/test_driver.c | 8 +- src/uml/uml_driver.c | 8 +- src/vbox/vbox_common.c | 4 +- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.c | 12 +-- src/xen/xm_internal.c | 14 +-- src/xenapi/xenapi_driver.c | 2 +- src/xenapi/xenapi_utils.c | 4 +- src/xenconfig/xen_common.c | 8 +- src/xenconfig/xen_sxpr.c | 9 +- .../qemuxml2argv-numatune-memnode.args | 2 +- 33 files changed, 248 insertions(+), 124 deletions(-) -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] qemu: Forbid seting maximum memory size with the API with NUMA enabled
NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case. Forbid use of the API in case NUMA is specified. --- src/qemu/qemu_driver.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b0dac7..e2abbe0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2307,6 +2307,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags VIR_DOMAIN_AFFECT_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); + +/* resizing memory with NUMA nodes specified doesn't work as there + * is no way to decrease the individual node sizes along */ +if (virDomainNumaGetNodeCount(persistentDef-numa) 0) { +virReportError(VIR_ERR_OPERATION_INVALID, %s, + _(maximum memory size of a domain with NUMA + nodes cannot be modified with this API)); +goto endjob; +} + persistentDef-mem.max_balloon = newmem; if (persistentDef-mem.cur_balloon newmem) persistentDef-mem.cur_balloon = newmem; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] qemu: command: Add helper to align memory sizes
The memory sizes in qemu are aligned up to 1 MiB boundaries. There are two places where this was done once for the total size and then for individual NUMA cell sizes. Add a function that will align the sizes in one place so that it's clear where the sizes are aligned. --- src/qemu/qemu_command.c | 7 +++ src/qemu/qemu_domain.c | 21 + src/qemu/qemu_domain.h | 2 ++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bf47702..7b83f9f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7177,9 +7177,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, /* using of -numa memdev= cannot be combined with -numa mem=, thus we * need to check which approach to use */ for (i = 0; i ncells; i++) { -unsigned long long cellmem = virDomainNumaGetNodeMemorySize(def-numa, i); -virDomainNumaSetNodeMemorySize(def-numa, i, VIR_ROUND_UP(cellmem, 1024)); - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, @@ -8313,13 +8310,15 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) 0) goto error; +if (qemuDomainAlignMemorySizes(def) 0) +goto error; + /* Set '-m MB' based on maxmem, because the lower 'memory' limit * is set post-startup using the balloon driver. If balloon driver * is not supported, then they're out of luck anyway. Update the * XML to reflect our rounding. */ virCommandAddArg(cmd, -m); -virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), 1024)); virCommandAddArgFormat(cmd, %llu, virDomainDefGetMemoryInitial(def) / 1024); if (def-mem.nhugepages !virDomainNumaGetNodeCount(def-numa)) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..c41b3d3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2832,3 +2832,24 @@ qemuDomObjEndAPI(virDomainObjPtr *vm) virObjectUnref(*vm); *vm = NULL; } + + +int +qemuDomainAlignMemorySizes(virDomainDefPtr def) +{ +unsigned long long mem; +size_t ncells = virDomainNumaGetNodeCount(def-numa); +size_t i; + +/* align NUMA cell sizes if relevant */ +for (i = 0; i ncells; i++) { +mem = virDomainNumaGetNodeMemorySize(def-numa, i); +virDomainNumaSetNodeMemorySize(def-numa, i, VIR_ROUND_UP(mem, 1024)); +} + +/* align initial memory size */ +mem = virDomainDefGetMemoryInitial(def); +virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024)); + +return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..faf4ee2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -414,4 +414,6 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, void qemuDomObjEndAPI(virDomainObjPtr *vm); +int qemuDomainAlignMemorySizes(virDomainDefPtr def); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] conf: numa: Add helper to count total memory size configured in NUMA
The total NUMA memory consists of the sum of individual NUMA node memory amounts. --- src/conf/numa_conf.c | 13 + src/conf/numa_conf.h | 1 + src/libvirt_private.syms | 1 + 3 files changed, 15 insertions(+) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index fb9884a..a3e8ab2 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -892,3 +892,16 @@ virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa, { numa-mem_nodes[node].mem = size; } + + +unsigned long long +virDomainNumaGetMemorySize(virDomainNumaPtr numa) +{ +size_t i; +unsigned long long ret = 0; + +for (i = 0; i numa-nmem_nodes; i++) +ret += numa-mem_nodes[i].mem; + +return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index e0cc62d..184d045 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -91,6 +91,7 @@ virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virDomainNumaPtr numa, size_t node); unsigned long long virDomainNumaGetNodeMemorySize(virDomainNumaPtr numa, size_t node); +unsigned long long virDomainNumaGetMemorySize(virDomainNumaPtr numa); /* diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a58b2b9..e2eb40c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -633,6 +633,7 @@ virNodeDeviceObjUnlock; virDomainNumaCheckABIStability; virDomainNumaEquals; virDomainNumaFree; +virDomainNumaGetMemorySize; virDomainNumaGetNodeCount; virDomainNumaGetNodeCpumask; virDomainNumaGetNodeMemoryAccessMode; -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Assign default SCSI controller model before checking attribute validity
If the qemu binary on x86 does not support lsi SCSI controller, but it supports virtio-scsi, we reject the virtio-specific attributes for no reason. Move the default controller assignment before the check. https://bugzilla.redhat.com/show_bug.cgi?id=1168849 --- src/qemu/qemu_command.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 743d6f0..882a97c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4353,10 +4353,15 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, int *nusbcontroller) { virBuffer buf = VIR_BUFFER_INITIALIZER; -int model; +int model = def-model; + +if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { +if ((qemuSetSCSIControllerModel(domainDef, qemuCaps, model)) 0) +return NULL; +} if (!(def-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI - def-model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) { + model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI)) { if (def-queues) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _('queues' is only supported by virtio-scsi controller)); @@ -4376,10 +4381,6 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, switch (def-type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: -model = def-model; -if ((qemuSetSCSIControllerModel(domainDef, qemuCaps, model)) 0) -return NULL; - switch (model) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: if (def-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] conf: Automatically use NUMA memory size in case NUMA is enabled
Use the NUMA total instead of the configured size both in XML and for uses in the code once NUMA is enabled for a domain. One test case change is necessary as the rounding of the individual cell sizes was not matching the rounding of the total size. --- src/conf/domain_conf.c| 6 ++ tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e95851a..a4cb930 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6895,6 +6895,12 @@ virDomainParseMemory(const char *xpath, unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def) { +unsigned long long ret; + +/* return NUMA memory size total in case numa is enabled */ +if ((ret = virDomainNumaGetMemorySize(def-numa)) 0) +return ret; + return def-mem.max_balloon; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args index 513d657..5dd7fcd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/kvm -S -M pc -m 24104 -smp 32 \ +/usr/bin/kvm -S -M pc -m 24105 -smp 32 \ -object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,\ policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] conf: Hoist validation of memory size into the post parse callback
Later patches will need to access the full definition to do check the memory size and thus the checking needs to be done after the whole definition including devices is known. --- src/conf/domain_conf.c | 41 - 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15d4fe0..6ef499d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3172,6 +3172,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } +if (def-mem.cur_balloon def-mem.max_balloon) { +/* Older libvirt could get into this situation due to + * rounding; if the discrepancy is less than 4MiB, we silently + * round down, otherwise we flag the issue. */ +if (VIR_DIV_UP(def-mem.cur_balloon, 4096) +VIR_DIV_UP(def-mem.max_balloon, 4096)) { +virReportError(VIR_ERR_XML_ERROR, + _(current memory '%lluk' exceeds + maximum '%lluk'), + def-mem.cur_balloon, def-mem.max_balloon); +return -1; +} else { +VIR_DEBUG(Truncating current %lluk to maximum %lluk, + def-mem.cur_balloon, def-mem.max_balloon); +def-mem.cur_balloon = def-mem.max_balloon; +} +} else if (def-mem.cur_balloon == 0) { +def-mem.cur_balloon = def-mem.max_balloon; +} + /* * Some really crazy backcompat stuff for consoles * @@ -13036,27 +13056,6 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(tmp); -if (def-mem.cur_balloon def-mem.max_balloon) { -/* Older libvirt could get into this situation due to - * rounding; if the discrepancy is less than 4MiB, we silently - * round down, otherwise we flag the issue. */ -if (VIR_DIV_UP(def-mem.cur_balloon, 4096) -VIR_DIV_UP(def-mem.max_balloon, 4096)) { -virReportError(VIR_ERR_XML_ERROR, - _(current memory '%lluk' exceeds - maximum '%lluk'), - def-mem.cur_balloon, def-mem.max_balloon); -goto error; -} else { -VIR_DEBUG(Truncating current %lluk to maximum %lluk, - def-mem.cur_balloon, def-mem.max_balloon); -def-mem.cur_balloon = def-mem.max_balloon; -} -} else if (def-mem.cur_balloon == 0) { -def-mem.cur_balloon = def-mem.max_balloon; -} - - if ((n = virXPathNodeSet(./memoryBacking/hugepages/page, ctxt, nodes)) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(cannot extract hugepages nodes)); -- 2.2.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] conf: Replace access to def-mem.max_balloon with accessor functions
As there are two possible approaches to define a domain's memory size - one used with legacy, non-NUMA VMs configured in the memory element and per-node based approach on NUMA machines - the user needs to make sure that both are specified correctly in the NUMA case. To avoid this burden on the user I'd like to replace the NUMA case with automatic totaling of the memory size. To achieve this I need to replace direct access to the virDomainMemtune's 'max_balloon' field with two separate getters depending on the desired size. The two sizes are needed as: 1) Startup memory size doesn't include memory modules in some hypervisors. 2) After startup these count as the usable memory size. Note that the comments for the functions are future aware and document state that will be present after a few later patches. --- src/conf/domain_conf.c | 66 ++-- src/conf/domain_conf.h | 4 +++ src/hyperv/hyperv_driver.c | 2 +- src/libvirt_private.syms | 3 ++ src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_driver.c | 8 ++--- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 10 +++--- src/lxc/lxc_fuse.c | 4 +-- src/lxc/lxc_native.c | 4 +-- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_sdk.c| 12 src/phyp/phyp_driver.c | 11 --- src/qemu/qemu_command.c | 18 +++ src/qemu/qemu_driver.c | 19 ++-- src/qemu/qemu_hotplug.c | 8 +++-- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 8 ++--- src/uml/uml_driver.c | 8 ++--- src/vbox/vbox_common.c | 4 +-- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.c| 12 src/xen/xm_internal.c| 14 - src/xenapi/xenapi_driver.c | 2 +- src/xenapi/xenapi_utils.c| 4 +-- src/xenconfig/xen_common.c | 8 +++-- src/xenconfig/xen_sxpr.c | 9 +++--- 28 files changed, 160 insertions(+), 90 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ef499d..e95851a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3172,24 +3172,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } -if (def-mem.cur_balloon def-mem.max_balloon) { +if (def-mem.cur_balloon virDomainDefGetMemoryCurrent(def)) { /* Older libvirt could get into this situation due to * rounding; if the discrepancy is less than 4MiB, we silently * round down, otherwise we flag the issue. */ if (VIR_DIV_UP(def-mem.cur_balloon, 4096) -VIR_DIV_UP(def-mem.max_balloon, 4096)) { +VIR_DIV_UP(virDomainDefGetMemoryCurrent(def), 4096)) { virReportError(VIR_ERR_XML_ERROR, _(current memory '%lluk' exceeds maximum '%lluk'), - def-mem.cur_balloon, def-mem.max_balloon); + def-mem.cur_balloon, + virDomainDefGetMemoryCurrent(def)); return -1; } else { VIR_DEBUG(Truncating current %lluk to maximum %lluk, - def-mem.cur_balloon, def-mem.max_balloon); -def-mem.cur_balloon = def-mem.max_balloon; + def-mem.cur_balloon, + virDomainDefGetMemoryCurrent(def)); +def-mem.cur_balloon = virDomainDefGetMemoryCurrent(def); } } else if (def-mem.cur_balloon == 0) { -def-mem.cur_balloon = def-mem.max_balloon; +def-mem.cur_balloon = virDomainDefGetMemoryCurrent(def); } /* @@ -6882,6 +6884,51 @@ virDomainParseMemory(const char *xpath, } +/** + * virDomainDefGetMemoryInitial: + * @def: domain definition + * + * Returns the size of the initial amount of guest memory. The initial amount + * is the memory size is either the configured amount in the memory element + * or the sum of memory sizes of NUMA nodes in case NUMA is enabled in @def. + */ +unsigned long long +virDomainDefGetMemoryInitial(virDomainDefPtr def) +{ +return def-mem.max_balloon; +} + + +/** + * virDomainDefSetMemoryInitial: + * @def: domain definition + * @size: size to set + * + * Sets the initial memory size in @def. + */ +void +virDomainDefSetMemoryInitial(virDomainDefPtr def, + unsigned long long size) +{ +def-mem.max_balloon = size; +} + + +/** + * virDomainDefGetMemoryCurrent: + * @def: domain definition + * + * Returns the current maximum memory size usable by the domain described by + * @def. This size is a sum of size returned by virDomainDefGetMemoryInitial + * and possible additional memory devices. + */ +unsigned long long +virDomainDefGetMemoryCurrent(virDomainDefPtr def) +{ +return
[libvirt] [PATCH] libvirt-guests: Allow time sync on guests resume
Well, imagine domains were running, and as the host went down, they were managesaved. Later, after some time, the host went up again and domains got restored. But without correct time. And depending on how long was the host shut off, it may take some time for ntp to sync the time too. But hey, wait a minute. We have an API just for that! So: 1) Introduce SYNC_TIME variable in libvirt-guests.sysconf to allow users control over the new functionality 2) Call 'virsh domtime --sync $dom' in the libvirt-guests script. Unfortunately, this is all-or-nothing approach (just like anything else with the script). Domains are required to have configured and running qemu-ga inside. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/libvirt-guests.sh.in | 6 ++ tools/libvirt-guests.sysconf | 7 +++ 2 files changed, 13 insertions(+) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 1b17bbe..21e39b0 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -171,7 +171,9 @@ start() { isfirst=true bypass= +sync_time=false test x$BYPASS_CACHE = x0 || bypass=--bypass-cache +test x$SYNC_TIME = x0 || sync_time=true while read uri list; do configured=false set -f @@ -206,6 +208,10 @@ start() { retval run_virsh $uri start $bypass $name \ /dev/null \ gettext done; echo +if $sync_time; then +retval run_virsh $uri domtime --sync $name \ +/dev/null +fi fi fi done diff --git a/tools/libvirt-guests.sysconf b/tools/libvirt-guests.sysconf index d1f2051..03e732f 100644 --- a/tools/libvirt-guests.sysconf +++ b/tools/libvirt-guests.sysconf @@ -39,3 +39,10 @@ # restoring guests, even though this may give slower operation for # some file systems. #BYPASS_CACHE=0 + +# If non-zero, try to sync guest time on a domain resume. Be aware, that +# this requires guest agent, which, moreover, has to run under supported +# system. For instance, qemu-ga doesn't support guest time synchronization +# on Windows guests, but Linux ones. By default, this piece of +# functionality is turned off. +#SYNC_TIME=1 -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: clarify nat range behavior
All the addresses from the range are used, not just those that are in use on the host. https://bugzilla.redhat.com/show_bug.cgi?id=1079917 --- docs/formatnetwork.html.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7cf3f69..6abed8f 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -200,6 +200,8 @@ pspan class=sinceSince 1.0.3/span it is possible to specify a public IPv4 address and port range to be used for the NAT by using the codelt;natgt;/code subelement. +Note that all addresses from the range are used, not just those +that are in use on the host. The address range is set with the codelt;addressgt;/code subelements and codestart/code and codestop/code attributes: -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libxl and non-absolute paths
On 18.02.2015 04:15, Jim Fehlig wrote: Stefan Bader wrote: Just recently we moved to libvirt 1.2.12 for the next release. Which brought up a few problems when working with configs which we and Debian used to have. A mild complaint towards the xml validation: it would be really nice of that would be a bit more specific about what exactly it complains. It took me a while to realize that Extra element os in interleave was trying to tell me that the string of the loader element within the os section was not an absolute path. The issue here is that with libxl, I think the goal was to rather allow the library to select the path prefix (like for pygrub where the full path got removed recently). But now the xml validation disagrees. This would go for bootloader for xenpv and loader (within os) for xenfv. And for emulator in the device section. Though for that things are a bit more complicated. The libxl driver now calls that with the help option and decides from the output whether this is the traditional xen forked qemu or the upstream qemu binary. Then it selects the device model depending on that outcome. It only sets the device_model_version (LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN vs LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL). device_model is directly VIR_STRDUP'ed from def-emulator. Ok, I did not word that correctly. The main caveat here is that to figure out the device_model_version it calls out to what is set in def-emulator. So it needs a full path. Up to libvirt 1.2.8 at least the contents of def-emulator were actually rather ignored since we ended up with qemu-dm which was not true at all. Not sure whether the libxl driver could query libxl for the path prefix. I asked about that too, but the Xen community preferred a build-time approach. Wei Liu added a pkgconfig module to Xen, enabling build-time detection of the paths commit babeca328413baebfdca366a5b17c06acf4295e8 Author: Wei Liu wei.l...@citrix.com Date: Fri Jan 9 14:32:18 2015 + libxl: provide xenlight.pc A pkg-config file for libxl. It also contains two variables (xenfirmwaredir and libexec_bin) so that tools that are very keen on knowing the locations of Xen binaries (say, libvirt) can use them to determine the location of the binaries. We are not yet making use of xenlight.pc in libvirt. But this work aims to improve reporting the correct paths in *capabilities*. It could also be used in the xl.cfg to domXML conversion code. Hm, ok. I guess there is upstream and upstream who might also be doing Debian (and derivatives of that). Since libxl is part of a version package the pathnames change between Xen versions. Maybe something to discuss about but not really here. Reality now causes the full path requirement to become a major pain in the butt. Right now the most straight forward way seems to move back to a full path for the emulator. Full paths are required as per the documentation. From http://libvirt.org/formatdomain.html#elementsOSBootloader The content of the |bootloader| element provides a fully qualified path to the bootloader executable in the host OS. Same for emulator. At least now, by using the standard qemu binary for everything, we got a predictable path that does not change with Xen versions. So its possible to force migrate over to put /usr/bin/qemu-system-i386 there. But for loader and bootloader, do you think it reasonable to change the templates from absFilePath to filePath? There's probably a fair bit of existing config with e.g. bootlaoderpygrub/bootloader that no longer validates. ATM, I don't have a good answer :-/. Is correct capabilities information and xl.cfg - libvirt.xml conversion, along with better error reporting enough? Should users change their non-validating domXML? Potentially this has to be magically converged in postparse of libxl. This is where I currently move from the completely lying qemu-dm to /usr/lib/qemu-system-i386. Though this will depend on using xenlight.pc and that depends on it being there. The change currently only is on the devel master branch. So presumably Xen 4.6 may have it. For the reality of now I guess the best way around the issues is to either disable validation completely or at least make that absFilePath-filePath conversion. None of that though is worthy of being upstream. -Stefan From libvirt's perspective, I think full paths, discovered from capabilities, should be required. I'd like to hear Daniel's opinion. He may have considered such cases when enabling the validation. Regards, Jim -Stefan --- libvirt-1.2.12.orig/docs/schemas/domaincommon.rng 2015-01-23 12:46:24. +++ libvirt-1.2.12/docs/schemas/domaincommon.rng2015-02-13 10:00:43.1616 @@ -258,7 +258,7 @@ /choice /attribute /optional -ref name=absFilePath/ +ref name=filePath/
[libvirt] [PATCH] man: moved virsh command cpu-models
The description of the virsh command 'cpu-models' was written in the wrong context (i.e. beside the domain states). This patch moves the command description just to the cpu related commands like 'cpu-baseline' and 'cpu-compare'. Signed-off-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com --- tools/virsh.pod | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 50de32c..b86c415 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -422,10 +422,6 @@ current domain is in. =over 4 -=item Bcpu-models Iarch - -Print the list of CPU models known for the specified architecture. - =item Brunning The domain is currently running on a CPU @@ -573,6 +569,10 @@ Icellno modifier can be used to narrow the modification down to a single host NUMA cell. On the other end of spectrum lies I--all which executes the modification on all NUMA cells. +=item Bcpu-models Iarch + +Print the list of CPU models known for the specified architecture. + =item Bcpu-baseline IFILE [I--features] Compute baseline CPU which will be supported by all host CPUs given in file. -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Adjust s390 test cases by removing usb device instances
Since usb devices are no longer created by default for s390 the s390 test cases need to be adjusted. Signed-off-by: Stefan Zimmermann s...@linux.vnet.ibm.com Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com --- tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args b/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args index 6aee214..4ea75de 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args @@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu \ -name QEMUGuest1 -S -M s390-virtio -m 214 -smp 1 -nographic \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi \ --boot c -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +-boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \ none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args b/tests/qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args index 10aecea..e939be4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args @@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -nodefconfig -nodefaults \ -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi \ --device virtio-serial-s390,id=virtio-serial0 -usb -drive \ +-device virtio-serial-s390,id=virtio-serial0 -drive \ file=/dev/HostVG/QEMUGuest1,if=none,id=drive-virtio-disk0 \ -device virtio-blk-s390,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \ -chardev pty,id=charconsole0 \ diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml index 9a609f8..54bb364 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml @@ -14,7 +14,6 @@ on_crashdestroy/on_crash devices emulator/usr/bin/qemu-kvm/emulator -controller type='usb' index='0' model='none'/ controller type='virtio-serial' index='0'/ console type='pty' target type='virtio' port='0'/ -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] s390: rework and prevent default usb controller
Stefan Zimmermann (3): Prevent default creation of usb controller on s390 and s390x Adjust s390 test cases by removing usb device instances Rework s390 architecture checking src/qemu/qemu_command.c | 4 +++- src/qemu/qemu_domain.c | 13 + tests/qemuxml2argvdata/qemuxml2argv-minimal-s390.args | 2 +- .../qemuxml2argv-s390-piix-controllers.args | 2 +- .../qemuxml2xmlout-s390-defaultconsole.xml | 1 - 5 files changed, 14 insertions(+), 8 deletions(-) -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt-guests: Allow time sync on guests resume
On Wed, Feb 18, 2015 at 15:31:27 +0100, Michal Privoznik wrote: Well, imagine domains were running, and as the host went down, they were managesaved. Later, after some time, the host went up again and domains got restored. But without correct time. And depending on how long was the host shut off, it may take some time for ntp to sync the time too. But hey, wait a minute. We have an API just for that! So: 1) Introduce SYNC_TIME variable in libvirt-guests.sysconf to allow users control over the new functionality 2) Call 'virsh domtime --sync $dom' in the libvirt-guests script. Unfortunately, this is all-or-nothing approach (just like anything else with the script). Domains are required to have configured and running qemu-ga inside. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/libvirt-guests.sh.in | 6 ++ tools/libvirt-guests.sysconf | 7 +++ 2 files changed, 13 insertions(+) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 1b17bbe..21e39b0 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -171,7 +171,9 @@ start() { isfirst=true bypass= +sync_time=false test x$BYPASS_CACHE = x0 || bypass=--bypass-cache +test x$SYNC_TIME = x0 || sync_time=true while read uri list; do configured=false set -f @@ -206,6 +208,10 @@ start() { retval run_virsh $uri start $bypass $name \ /dev/null \ gettext done; echo +if $sync_time; then +retval run_virsh $uri domtime --sync $name \ I think 'retva' shouldn't be used here. Time sync is very prone to fail as it uses the guest agent and we should not fail to start the service for this operation that I'd consider best effort. +/dev/null +fi fi fi done Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt-guests: Allow time sync on guests resume
On Wed, Feb 18, 2015 at 04:52:48PM +0100, Peter Krempa wrote: On Wed, Feb 18, 2015 at 15:31:27 +0100, Michal Privoznik wrote: Well, imagine domains were running, and as the host went down, they were managesaved. Later, after some time, the host went up again and domains got restored. But without correct time. And depending on how long was the host shut off, it may take some time for ntp to sync the time too. But hey, wait a minute. We have an API just for that! So: 1) Introduce SYNC_TIME variable in libvirt-guests.sysconf to allow users control over the new functionality 2) Call 'virsh domtime --sync $dom' in the libvirt-guests script. Unfortunately, this is all-or-nothing approach (just like anything else with the script). Domains are required to have configured and running qemu-ga inside. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tools/libvirt-guests.sh.in | 6 ++ tools/libvirt-guests.sysconf | 7 +++ 2 files changed, 13 insertions(+) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 1b17bbe..21e39b0 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -171,7 +171,9 @@ start() { isfirst=true bypass= +sync_time=false test x$BYPASS_CACHE = x0 || bypass=--bypass-cache +test x$SYNC_TIME = x0 || sync_time=true while read uri list; do configured=false set -f @@ -206,6 +208,10 @@ start() { retval run_virsh $uri start $bypass $name \ /dev/null \ gettext done; echo +if $sync_time; then +retval run_virsh $uri domtime --sync $name \ I think 'retva' shouldn't be used here. Time sync is very prone to fail as it uses the guest agent and we should not fail to start the service for this operation that I'd consider best effort. Yeah I thin that makes sense, as we don't even know if the agent is installed in all guests. We just want it to work in as many as possible without breaking startup entirely. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] libvirt-guests: Allow time sync on guests resume
Well, imagine domains were running, and as the host went down, they were managesaved. Later, after some time, the host went up again and domains got restored. But without correct time. And depending on how long was the host shut off, it may take some time for ntp to sync the time too. But hey, wait a minute. We have an API just for that! So: 1) Introduce SYNC_TIME variable in libvirt-guests.sysconf to allow users control over the new functionality 2) Call 'virsh domtime --sync $dom' in the libvirt-guests script. Unfortunately, this is all-or-nothing approach (just like anything else with the script). Domains are required to have configured and running qemu-ga inside. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- diff to v1: -ignore the return value of domtime command tools/libvirt-guests.sh.in | 5 + tools/libvirt-guests.sysconf | 7 +++ 2 files changed, 12 insertions(+) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 1b17bbe..9aa06fa 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -171,7 +171,9 @@ start() { isfirst=true bypass= +sync_time=false test x$BYPASS_CACHE = x0 || bypass=--bypass-cache +test x$SYNC_TIME = x0 || sync_time=true while read uri list; do configured=false set -f @@ -206,6 +208,9 @@ start() { retval run_virsh $uri start $bypass $name \ /dev/null \ gettext done; echo +if $sync_time; then +run_virsh $uri domtime --sync $name /dev/null +fi fi fi done diff --git a/tools/libvirt-guests.sysconf b/tools/libvirt-guests.sysconf index d1f2051..03e732f 100644 --- a/tools/libvirt-guests.sysconf +++ b/tools/libvirt-guests.sysconf @@ -39,3 +39,10 @@ # restoring guests, even though this may give slower operation for # some file systems. #BYPASS_CACHE=0 + +# If non-zero, try to sync guest time on a domain resume. Be aware, that +# this requires guest agent, which, moreover, has to run under supported +# system. For instance, qemu-ga doesn't support guest time synchronization +# on Windows guests, but Linux ones. By default, this piece of +# functionality is turned off. +#SYNC_TIME=1 -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); Why don't we just not register the reload handler until we get to the point where it is safe. eg register it only after virStateInitialize is complete, and unregister it before we call virStateCleanup. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Prevent default creation of usb controller on s390 and s390x
Since s390 does not support usb the default creation of a usb controller for a domain should not occur. Signed-off-by: Stefan Zimmermann s...@linux.vnet.ibm.com Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com --- src/qemu/qemu_command.c | 4 +++- src/qemu/qemu_domain.c | 6 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8e864ab..d9ed96a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9001,7 +9001,9 @@ qemuBuildCommandLine(virConnectPtr conn, } } -if (usbcontroller == 0 !qemuDomainMachineIsQ35(def)) +if (usbcontroller == 0 +!qemuDomainMachineIsQ35(def) +!ARCH_IS_S390(def-os.arch)) virCommandAddArg(cmd, -usb); for (i = 0; i def-nhubs; i++) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..bd9d4f2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -979,6 +979,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, case VIR_ARCH_SH4EB: addPCIRoot = true; break; +case VIR_ARCH_S390: +addDefaultUSB = false; +break; +case VIR_ARCH_S390X: +addDefaultUSB = false; +break; default: break; } -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Rework s390 architecture checking
Making use of the ARCH_IS_S390 macro introduced with e808357528d8be1ebc3970424b4a7b7c04eda2b6 Signed-off-by: Stefan Zimmermann s...@linux.vnet.ibm.com Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com --- src/qemu/qemu_domain.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bd9d4f2..ac5ca74 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1052,8 +1052,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, static const char * qemuDomainDefaultNetModel(const virDomainDef *def) { -if (def-os.arch == VIR_ARCH_S390 || -def-os.arch == VIR_ARCH_S390X) +if (ARCH_IS_S390(def-os.arch)) return virtio; if (def-os.arch == VIR_ARCH_ARMV7L || @@ -1132,7 +1131,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev-type == VIR_DOMAIN_DEVICE_CHR dev-data.chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE dev-data.chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE -(def-os.arch == VIR_ARCH_S390 || def-os.arch == VIR_ARCH_S390X)) +ARCH_IS_S390(def-os.arch)) dev-data.chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; /* set the default USB model to none for s390 unless an address is found */ @@ -1140,7 +1139,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_USB dev-data.controller-model == -1 dev-data.controller-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE -(def-os.arch == VIR_ARCH_S390 || def-os.arch == VIR_ARCH_S390X)) +ARCH_IS_S390(def-os.arch)) dev-data.controller-model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE; /* auto generate unix socket path */ -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start
Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- daemon/libvirtd.c | 5 + 1 file changed, 5 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..835e7dc 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] DO NOT PUSH: reproducer for segfault patch
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/qemu/qemu_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1bbbe9b..b6280a5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -644,6 +644,8 @@ qemuStateInitialize(bool privileged, return -1; } +kill(getpid(), SIGHUP); + qemu_driver-inhibitCallback = callback; qemu_driver-inhibitOpaque = opaque; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: clarify nat range behavior
On 02/18/2015 07:39 AM, Ján Tomko wrote: All the addresses from the range are used, not just those that are in use on the host. https://bugzilla.redhat.com/show_bug.cgi?id=1079917 --- docs/formatnetwork.html.in | 2 ++ 1 file changed, 2 insertions(+) ACK diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7cf3f69..6abed8f 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -200,6 +200,8 @@ pspan class=sinceSince 1.0.3/span it is possible to specify a public IPv4 address and port range to be used for the NAT by using the codelt;natgt;/code subelement. +Note that all addresses from the range are used, not just those +that are in use on the host. The address range is set with the codelt;addressgt;/code subelements and codestart/code and codestop/code attributes: -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] daemon: Fix segfault by reloading daemon right after start
Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. In addition set driversInitialized before we enter virStateCleanup to ignore reload request while we are shutting down. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- diff to v1: - ignore reload also during virStateCleanup daemon/libvirtd.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 86accaa..2366d63 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED, siginfo_t *sig ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { +if (!driversInitialized) { +VIR_WARN(Drivers are not initialized, reload ignored); +return; +} + VIR_INFO(Reloading configuration on SIGHUP); virHookCall(VIR_HOOK_DRIVER_DAEMON, -, VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, SIGHUP, NULL, NULL); @@ -1519,8 +1524,10 @@ int main(int argc, char **argv) { daemonConfigFree(config); -if (driversInitialized) +if (driversInitialized) { +driversInitialized = false; virStateCleanup(); +} return ret; } -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: clarify nat range behavior
On Wed, Feb 18, 2015 at 15:39:08 +0100, Ján Tomko wrote: All the addresses from the range are used, not just those that are in use on the host. https://bugzilla.redhat.com/show_bug.cgi?id=1079917 --- docs/formatnetwork.html.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 7cf3f69..6abed8f 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -200,6 +200,8 @@ pspan class=sinceSince 1.0.3/span it is possible to specify a public IPv4 address and port range to be used for the NAT by using the codelt;natgt;/code subelement. +Note that all addresses from the range are used, not just those +that are in use on the host. The address range is set with the codelt;addressgt;/code subelements and codestart/code and codestop/code attributes: ACK, Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] Make -boot arg generation more readable
If we combine the boot order on the command line with other boot options, we prepend order= in front of it. Instead of checking if the number of added arguments is between 0 and 2, separate the buffers for boot order and options and prepend boot order only if both buffers are not empty. --- src/qemu/qemu_command.c | 37 ++--- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3658d5f..f13dbda 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8208,6 +8208,8 @@ qemuBuildCommandLine(virConnectPtr conn, virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer boot_buf = VIR_BUFFER_INITIALIZER; +virBuffer boot_order = VIR_BUFFER_INITIALIZER; +char *boot_order_str = NULL, *boot_opts_str = NULL; int boot_nparams = 0; VIR_DEBUG(conn=%p driver=%p def=%p mon=%p json=%d @@ -8772,10 +8774,12 @@ qemuBuildCommandLine(virConnectPtr conn, } boot[def-os.nBootDevs] = '\0'; -virBufferAsprintf(boot_buf, %s, boot); -boot_nparams++; +virBufferAsprintf(boot_order, %s, boot); } +if (virBufferCheckError(boot_order) 0) +goto error; + if (def-os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) @@ -8829,23 +8833,25 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAddLit(boot_buf, strict=on); } -if (boot_nparams 0) { -virCommandAddArg(cmd, -boot); +if (virBufferCheckError(boot_buf) 0) +goto error; -if (virBufferCheckError(boot_buf) 0) -goto error; +boot_order_str = virBufferContentAndReset(boot_order); +boot_opts_str = virBufferContentAndReset(boot_buf); +if (boot_order_str || boot_opts_str) { +virCommandAddArg(cmd, -boot); -if (boot_nparams 2 || emitBootindex) { -virCommandAddArgBuffer(cmd, boot_buf); -virBufferFreeAndReset(boot_buf); -} else { -char *str = virBufferContentAndReset(boot_buf); -virCommandAddArgFormat(cmd, - order=%s, - str); -VIR_FREE(str); +if (boot_order_str boot_opts_str) { +virCommandAddArgFormat(cmd, order=%s,%s, + boot_order_str, boot_opts_str); +} else if (boot_order_str) { +virCommandAddArg(cmd, boot_order_str); +} else if (boot_opts_str) { +virCommandAddArg(cmd, boot_opts_str); } } +VIR_FREE(boot_order_str); +VIR_FREE(boot_opts_str); if (def-os.kernel) virCommandAddArgList(cmd, -kernel, def-os.kernel, NULL); @@ -10335,6 +10341,7 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd; error: +virBufferFreeAndReset(boot_order); virBufferFreeAndReset(boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] Remove bootloader option from QEMU
It was only supported by xenner, for which we removed support in commit de9be0a. Remove the code generating this command line option, refuse to parse it and delete the outdated tests. https://bugzilla.redhat.com/show_bug.cgi?id=1176050 --- src/qemu/qemu_command.c| 233 ++--- src/qemu/qemu_domain.c | 6 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 27 --- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 34 --- tests/qemuxml2argvtest.c | 2 - tests/qemuxml2xmltest.c| 2 - 8 files changed, 120 insertions(+), 194 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 743d6f0..b522bdc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8207,6 +8207,8 @@ qemuBuildCommandLine(virConnectPtr conn, }; virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +virBuffer boot_buf = VIR_BUFFER_INITIALIZER; +int boot_nparams = 0; VIR_DEBUG(conn=%p driver=%p def=%p mon=%p json=%d qemuCaps=%p migrateFrom=%s migrateFD=%d @@ -8728,148 +8730,140 @@ qemuBuildCommandLine(virConnectPtr conn, def-pm.s4 == VIR_TRISTATE_BOOL_NO); } -if (!def-os.bootloader) { -int boot_nparams = 0; -virBuffer boot_buf = VIR_BUFFER_INITIALIZER; -/* - * We prefer using explicit bootindex=N parameters for predictable - * results even though domain XML doesn't use per device boot elements. - * However, we can't use bootindex if boot menu was requested. +/* + * We prefer using explicit bootindex=N parameters for predictable + * results even though domain XML doesn't use per device boot elements. + * However, we can't use bootindex if boot menu was requested. + */ +if (!def-os.nBootDevs) { +/* def-os.nBootDevs is guaranteed to be 0 unless per-device boot + * configuration is used */ -if (!def-os.nBootDevs) { -/* def-os.nBootDevs is guaranteed to be 0 unless per-device boot - * configuration is used - */ -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(hypervisor lacks deviceboot feature)); -goto error; -} -emitBootindex = true; -} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) - (def-os.bootmenu != VIR_TRISTATE_BOOL_YES || -!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { -emitBootindex = true; -} - -if (!emitBootindex) { -char boot[VIR_DOMAIN_BOOT_LAST+1]; - -for (i = 0; i def-os.nBootDevs; i++) { -switch (def-os.bootDevs[i]) { -case VIR_DOMAIN_BOOT_CDROM: -boot[i] = 'd'; -break; -case VIR_DOMAIN_BOOT_FLOPPY: -boot[i] = 'a'; -break; -case VIR_DOMAIN_BOOT_DISK: -boot[i] = 'c'; -break; -case VIR_DOMAIN_BOOT_NET: -boot[i] = 'n'; -break; -default: -boot[i] = 'c'; -break; -} -} -boot[def-os.nBootDevs] = '\0'; - -virBufferAsprintf(boot_buf, %s, boot); -boot_nparams++; +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(hypervisor lacks deviceboot feature)); +goto error; } +emitBootindex = true; +} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) + (def-os.bootmenu != VIR_TRISTATE_BOOL_YES || +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { +emitBootindex = true; +} -if (def-os.bootmenu) { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { -if (boot_nparams++) -virBufferAddChar(boot_buf, ','); +if (!emitBootindex) { +char boot[VIR_DOMAIN_BOOT_LAST+1]; -if (def-os.bootmenu == VIR_TRISTATE_BOOL_YES) -virBufferAddLit(boot_buf, menu=on); -else -
[libvirt] [PATCH 4/5] Rename boot_buf to boot_opts
--- src/qemu/qemu_command.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f13dbda..0b5fb72 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8207,7 +8207,7 @@ qemuBuildCommandLine(virConnectPtr conn, }; virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); -virBuffer boot_buf = VIR_BUFFER_INITIALIZER; +virBuffer boot_opts = VIR_BUFFER_INITIALIZER; virBuffer boot_order = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; int boot_nparams = 0; @@ -8783,12 +8783,12 @@ qemuBuildCommandLine(virConnectPtr conn, if (def-os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) -virBufferAddChar(boot_buf, ','); +virBufferAddChar(boot_opts, ','); if (def-os.bootmenu == VIR_TRISTATE_BOOL_YES) -virBufferAddLit(boot_buf, menu=on); +virBufferAddLit(boot_opts, menu=on); else -virBufferAddLit(boot_buf, menu=off); +virBufferAddLit(boot_opts, menu=off); } else { /* We cannot emit an error when bootmenu is enabled but * unsupported because of backward compatibility */ @@ -8806,9 +8806,9 @@ qemuBuildCommandLine(virConnectPtr conn, } if (boot_nparams++) -virBufferAddChar(boot_buf, ','); +virBufferAddChar(boot_opts, ','); -virBufferAsprintf(boot_buf, +virBufferAsprintf(boot_opts, reboot-timeout=%d, def-os.bios.rt_delay); } @@ -8822,22 +8822,22 @@ qemuBuildCommandLine(virConnectPtr conn, } if (boot_nparams++) -virBufferAddChar(boot_buf, ','); +virBufferAddChar(boot_opts, ','); -virBufferAsprintf(boot_buf, splash-time=%u, def-os.bm_timeout); +virBufferAsprintf(boot_opts, splash-time=%u, def-os.bm_timeout); } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { if (boot_nparams++) -virBufferAddChar(boot_buf, ','); -virBufferAddLit(boot_buf, strict=on); +virBufferAddChar(boot_opts, ','); +virBufferAddLit(boot_opts, strict=on); } -if (virBufferCheckError(boot_buf) 0) +if (virBufferCheckError(boot_opts) 0) goto error; boot_order_str = virBufferContentAndReset(boot_order); -boot_opts_str = virBufferContentAndReset(boot_buf); +boot_opts_str = virBufferContentAndReset(boot_opts); if (boot_order_str || boot_opts_str) { virCommandAddArg(cmd, -boot); @@ -10342,7 +10342,7 @@ qemuBuildCommandLine(virConnectPtr conn, error: virBufferFreeAndReset(boot_order); -virBufferFreeAndReset(boot_buf); +virBufferFreeAndReset(boot_opts); virObjectUnref(cfg); /* free up any resources in the network driver * but don't overwrite the original error */ -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] Remove code handling the QEMU_CAPS_DOMID capability
This option is xenner-only, and we dropped support for xenner in commit de9be0a. --- src/qemu/qemu_command.c | 5 + tests/qemuxml2argvtest.c | 5 + tests/qemuxmlnstest.c| 5 + 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b522bdc..3658d5f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8408,10 +8408,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (def-virtType == VIR_DOMAIN_VIRT_XEN || STREQ(def-os.type, xen) || STREQ(def-os.type, linux)) { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DOMID)) { -virCommandAddArg(cmd, -domid); -virCommandAddArgFormat(cmd, %d, def-id); -} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_XEN_DOMID)) { +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_XEN_DOMID)) { virCommandAddArg(cmd, -xen-attach); virCommandAddArg(cmd, -xen-domid); virCommandAddArgFormat(cmd, %d, def-id); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1071dba..ba1813c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -289,10 +289,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } -if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DOMID)) -vmdef-id = 6; -else -vmdef-id = -1; +vmdef-id = -1; memset(monitor_chr, 0, sizeof(monitor_chr)); monitor_chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index a068135..cc290cd 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -86,10 +86,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; } -if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DOMID)) -vmdef-id = 6; -else -vmdef-id = -1; +vmdef-id = -1; memset(monitor_chr, 0, sizeof(monitor_chr)); monitor_chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] daemon: Fix segfault by reloading daemon right after start
On Wed, Feb 18, 2015 at 05:34:39PM +0100, Pavel Hrdina wrote: Libvirt could crash with segfault if user issue service reload right after service start. One possible way to crash libvirt is to run reload during initialization of QEMU driver. It could happen when qemu driver will initialize qemu_driver_lock but don't have a time to set it's config and the SIGHUP arrives. The reload handler tries to get qemu_drv-config during virStorageAutostart and dereference it which ends with segfault. Let's ignore all reload requests until all drivers are initialized. In addition set driversInitialized before we enter virStateCleanup to ignore reload request while we are shutting down. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- diff to v1: - ignore reload also during virStateCleanup daemon/libvirtd.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Boot options cleanup
Remove the -bootloader option from the QEMU driver, for https://bugzilla.redhat.com/show_bug.cgi?id=1176050 as it was only supported by xenner. Remove the -domid option, also xenner-only. Cleanup -boot command line generation. Ján Tomko (5): Remove bootloader option from QEMU Remove code handling the QEMU_CAPS_DOMID capability Make -boot arg generation more readable Rename boot_buf to boot_opts Use virBufferTrim when generating boot options src/qemu/qemu_command.c| 240 ++--- src/qemu/qemu_domain.c | 6 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 27 --- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 34 --- tests/qemuxml2argvtest.c | 7 +- tests/qemuxml2xmltest.c| 2 - tests/qemuxmlnstest.c | 5 +- 9 files changed, 122 insertions(+), 209 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] Use virBufferTrim when generating boot options
Instead of tracking the number of added parameters, add a comma at the end of each one unconditionally and trim the trailing one at the end. --- src/qemu/qemu_command.c | 27 --- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5fb72..06a9e54 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8210,7 +8210,6 @@ qemuBuildCommandLine(virConnectPtr conn, virBuffer boot_opts = VIR_BUFFER_INITIALIZER; virBuffer boot_order = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; -int boot_nparams = 0; VIR_DEBUG(conn=%p driver=%p def=%p mon=%p json=%d qemuCaps=%p migrateFrom=%s migrateFD=%d @@ -8782,13 +8781,10 @@ qemuBuildCommandLine(virConnectPtr conn, if (def-os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { -if (boot_nparams++) -virBufferAddChar(boot_opts, ','); - if (def-os.bootmenu == VIR_TRISTATE_BOOL_YES) -virBufferAddLit(boot_opts, menu=on); +virBufferAddLit(boot_opts, menu=on,); else -virBufferAddLit(boot_opts, menu=off); +virBufferAddLit(boot_opts, menu=off,); } else { /* We cannot emit an error when bootmenu is enabled but * unsupported because of backward compatibility */ @@ -8805,11 +8801,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } -if (boot_nparams++) -virBufferAddChar(boot_opts, ','); - virBufferAsprintf(boot_opts, - reboot-timeout=%d, + reboot-timeout=%d,, def-os.bios.rt_delay); } @@ -8821,17 +8814,13 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } -if (boot_nparams++) -virBufferAddChar(boot_opts, ','); - -virBufferAsprintf(boot_opts, splash-time=%u, def-os.bm_timeout); +virBufferAsprintf(boot_opts, splash-time=%u,, def-os.bm_timeout); } -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { -if (boot_nparams++) -virBufferAddChar(boot_opts, ','); -virBufferAddLit(boot_opts, strict=on); -} +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) +virBufferAddLit(boot_opts, strict=on,); + +virBufferTrim(boot_opts, ,, -1); if (virBufferCheckError(boot_opts) 0) goto error; -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] libxl: add tablet/mouse input device support
Marek Marczykowski-Górecki wrote: Signed-off-by: Marek Marczykowski-Górecki marma...@invisiblethingslab.com --- Changes in v2: - rebase on 1.2.12+ - multiple devices support src/libxl/libxl_conf.c | 48 1 file changed, 48 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4730585..b1131ea 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -815,6 +815,54 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } +if (def-ninputs) { +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +if (VIR_ALLOC_N(b_info-u.hvm.usbdevice_list, def-ninputs+1) 0) +return -1; +#else +if (def-ninputs 1) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(libxenlight supports only one input device)); +return -1; +} +#endif +for (i = 0; i def-ninputs; i++) { +if (def-inputs[i]-bus != VIR_DOMAIN_INPUT_BUS_USB) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(libxenlight supports only USB input)); +return -1; +} +} +for (i = 0; i def-ninputs; i++) { +switch (def-inputs[i]-type) { +case VIR_DOMAIN_INPUT_TYPE_MOUSE: +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +if (VIR_STRDUP(b_info-u.hvm.usbdevice_list[i], mouse) 0) +return -1; +#else +VIR_FREE(b_info-u.hvm.usbdevice); +if (VIR_STRDUP(b_info-u.hvm.usbdevice, mouse) 0) +return -1; +#endif +break; +case VIR_DOMAIN_INPUT_TYPE_TABLET: +#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST +if (VIR_STRDUP(b_info-u.hvm.usbdevice_list[i], tablet) 0) +return -1; +#else +VIR_FREE(b_info-u.hvm.usbdevice); +if (VIR_STRDUP(b_info-u.hvm.usbdevice, tablet) 0) +return -1; +#endif One of the #ifdef could be dropped with something like for (i=0; i def-ninputs; i++) { char *temp; #ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST temp = b_info-u.hvm.usbdevice_list[i]; #else temp = b_info-u.hvm.usbdevice; #endif ... VIR_STRDUP(temp, mouse); ... } Also, could you look into adding support for usbdevice_list to the parsing/formating code in src/xenconfig? Regards, Jim +break; +default: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, +_(Unknown input device type)); +return -1; +} +} +} + /* * The following comment and calculation were taken directly from * libxenlight's internal function libxl_get_required_shadow_memory(): -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list