- Devel - libvirt List Archives

[libvirt] [PATCH] build: inform libtool of m4 directory
by Eric Blake 17 Feb '10

17 Feb '10

Silence a libtoolize warning. * configure.ac (AC_CONFIG_MACRO_DIR): Add. --- Libtool spits out this warning during ./autogen.sh: | libtoolize: Consider adding `AC_CONFIG_MACRO_DIR([m4])' to configure.ac and | libtoolize: rerunning libtoolize, to keep the correct libtool macros in-tree. I've checked that there is no significant difference in the generated files when applying this patch to silence the warning, and that autoconf 2.59 supplied this macro. configure.ac | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 1c7c24c..743a357 100644 --- a/configure.ac +++ b/configure.ac @@ -4,6 +4,7 @@ AC_INIT([libvirt], [0.7.6]) AC_CONFIG_SRCDIR([src/libvirt.c]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) +AC_CONFIG_MACRO_DIR([m4]) dnl Make automake keep quiet about wildcards & other GNUmake-isms AM_INIT_AUTOMAKE([-Wno-portability]) -- 1.6.6

2 1

[libvirt] [PATCH] Fix macvtap device tear down problem on virsh destroy
by Stefan Berger 17 Feb '10

17 Feb '10

This patch fixes the problem with the tear down of the macvtap device when issuing a 'virsh destroy' by moving the tear down block past the point of killing the Qemu process. It seems necessary to loop at least once in the case of 'virsh destory' since the device seems to be busy for a while after the Qemu process has been killed. This also still properly tears down the macvtap device when the VM is 'virsh shutdown'ed or halted from inside. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

2 5

[libvirt] [PATCH v2 0/9] New public API for computing baseline CPU
by Jiri Denemark 17 Feb '10

17 Feb '10

Version 2 changes: - rebased (mainly for conn argument removal) - swapped array and number of elements in array parameters - fixed absolute paths in generated RPC code - added python bindings for virConnectBaselineCPU - fixed uninitialized variable in cpuDecode, which wasn't revealed by gcc :-( This series introduces a new virConnectBaselineCPU() API call for computing baseline CPU model from a set of host CPU definitions. The call accepts a list of <cpu>...</cpu> xml definitions taken from host capabilities and returns an xml description of a guest CPU which is supported by all hosts. virsh is little bit less picky and accepts a file containing host CPU definitions. It doesn't care what is outside of the <cpu/> tag, which allows for concatenating outputs of virsh capabilities from several hosts and feeding the result to virsh cpu-baseline. This series can also be found on cpu-baseline branch of git://gitorious.org/~jirka/libvirt/jirka-staging.git In practice it works like this: # virsh cpu-baseline /dev/stdin >guest-cpu <<EOF <capabilities> <host> <cpu> <arch>x86_64</arch> <model>core2duo</model> <topology sockets='1' cores='2' threads='1'/> <feature name='lahf_lm'/> <feature name='xtpr'/> <feature name='cx16'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> </cpu> </host> </capabilities> <cpu> <arch>i686</arch> <model>pentiumpro</model> <topology sockets='1' cores='2' threads='1'/> <feature name='vme'/> <feature name='pse36'/> <feature name='pni'/> <feature name='monitor'/> <feature name='ssse3'/> </cpu> <cpu> <arch>x86_64</arch> <model>phenom</model> </cpu> EOF # cat guest-cpu <cpu match='exact'> <model>qemu32</model> <feature policy='require' name='monitor'/> <feature policy='require' name='pse36'/> </cpu> Jiri Denemark (9): Functions for computing baseline CPU from a set of host CPUs Implement cpuArchBaseline in generic CPU driver Implement cpuArchBaseline in x86 CPU driver virConnectBaselineCPU public API Internal driver API for virConnectBaselineCPU virConnectBaselineCPU public API implementation Wire protocol format and dispatcher for virConnectBaselineCPU Implement cpuBaseline in remote and qemu drivers cpu-baseline command for virsh daemon/remote.c | 25 +++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 ++ include/libvirt/libvirt.h.in | 18 +++++ python/generator.py | 1 + python/libvirt-override-api.xml | 7 ++ python/libvirt-override.c | 52 +++++++++++++++ src/cpu/cpu.c | 123 ++++++++++++++++++++++++++++++++++- src/cpu/cpu.h | 21 ++++++- src/cpu/cpu_generic.c | 104 +++++++++++++++++++++++++++++- src/cpu/cpu_x86.c | 88 ++++++++++++++++++++++++- src/driver.h | 6 ++ src/esx/esx_driver.c | 1 + src/libvirt.c | 57 ++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 1 + src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 14 ++++ src/remote/remote_driver.c | 32 +++++++++ src/remote/remote_protocol.c | 22 ++++++ src/remote/remote_protocol.h | 20 ++++++ src/remote/remote_protocol.x | 18 +++++- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + tools/virsh.c | 72 ++++++++++++++++++++ tools/virsh.pod | 8 ++ 33 files changed, 709 insertions(+), 6 deletions(-)

4 32

[libvirt] [PATCH] vbox_tmpl.c: avoid leak on OOM error path
by Jim Meyering 17 Feb '10

17 Feb '10

We have to free def here, rather than after "cleanup:", because "cleanup:" is on the successful-return path. >From 2c7800c3560f03fd77e4a458cb0e557165faed3f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 17:54:15 +0100 Subject: [PATCH] vbox_tmpl.c: avoid leak on OOM error path * src/vbox/vbox_tmpl.c (vboxDomainDumpXML): Free def upon virAsprintf failure. --- src/vbox/vbox_tmpl.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 68dffd2..1fb8d56 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6059,14 +6059,15 @@ static char *vboxNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSE if (VIR_ALLOC(def) < 0) { virReportOOMError(); goto cleanup; } if (virAsprintf(&networkNameUtf8, "HostInterfaceNetworking-%s", network->name) < 0) { virReportOOMError(); + VIR_FREE(def); goto cleanup; } PRUnichar *networkInterfaceNameUtf16 = NULL; IHostNetworkInterface *networkInterface = NULL; VBOX_UTF8_TO_UTF16(network->name, &networkInterfaceNameUtf16); -- 1.7.0.181.g41533

2 6

Re: [libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot 16 Feb '10

16 Feb '10

Yep, here is what I get (see at the end) : 14:06:22.071: debug : do_open:1110 : name "qemu:///system" to URI components: scheme qemu opaque (null) authority (null) server (null) user (null) port 0 path /system 14:06:22.071: debug : do_open:1120 : trying driver 0 (Test) ... 14:06:22.071: debug : do_open:1126 : driver 0 Test returned DECLINED 14:06:22.071: debug : do_open:1120 : trying driver 1 (OPENVZ) ... 14:06:22.071: debug : do_open:1126 : driver 1 OPENVZ returned DECLINED 14:06:22.071: debug : do_open:1120 : trying driver 2 (VBOX) ... 14:06:22.071: debug : do_open:1126 : driver 2 VBOX returned DECLINED 14:06:22.071: debug : do_open:1120 : trying driver 3 (remote) ... 14:06:22.071: debug : do_open:1126 : driver 3 remote returned DECLINED 14:06:22.071: debug : do_open:1120 : trying driver 4 (QEMU) ... 14:06:22.071: debug : do_open:1126 : driver 4 QEMU returned SUCCESS 14:06:22.071: debug : do_open:1146 : network driver 0 Test returned DECLINED 14:06:22.071: debug : do_open:1146 : network driver 1 VBOX returned DECLINED 14:06:22.071: debug : do_open:1146 : network driver 2 remote returned DECLINED 14:06:22.071: debug : do_open:1146 : network driver 3 Network returned SUCCESS 14:06:22.071: debug : do_open:1165 : interface driver 0 Test returned DECLINED 14:06:22.071: debug : do_open:1165 : interface driver 1 remote returned DECLINED 14:06:22.072: debug : do_open:1185 : storage driver 0 Test returned DECLINED 14:06:22.072: debug : do_open:1185 : storage driver 1 VBOX returned DECLINED 14:06:22.072: debug : do_open:1185 : storage driver 2 remote returned DECLINED 14:06:22.072: debug : do_open:1185 : storage driver 3 storage returned SUCCESS 14:06:22.072: debug : do_open:1205 : node driver 0 Test returned DECLINED 14:06:22.072: debug : do_open:1205 : node driver 1 remote returned DECLINED 14:06:22.072: debug : do_open:1232 : secret driver 0 Test returned DECLINED 14:06:22.072: debug : do_open:1232 : secret driver 1 remote returned DECLINED 14:06:22.072: debug : do_open:1232 : secret driver 2 secret returned SUCCESS 14:06:22.072: debug : virConnectClose:1381 : conn=0x1eaf0b0 I think the times when I get those lines matches with my "virsh nodedev-list" requests (or something like this : virt-manager tries to create a vm, add hardware...). Does this help? Frederic. ----- "Dave Allan" <dallan(a)redhat.com> a écrit : > On 02/16/2010 03:00 AM, Frédéric Grelot wrote: > >> Daniel was referring to the libvirt udev driver--the libvirt code > that > >> > >> interacts with udev--not to udev itself, which is what you're > testing > >> > >> with udevadm. > > > > Ok, sorry... At least I ensured that udev itself is on... > > > >> In any of your logs when you have libvirt debug logging > >> turned on, do you see the message: > >> > >> Registering udev node device backend > > > > Nope, I checked in /var/log/* and libvirt.log in debug mode (which, > after 1 1/2 day of "debug" activity enabled, is already 2Gb!) > > There is no mention of udev. I think it means that the udev driver > is not loaded in libvirt? How could enable it then? (or produce some > message telling me why it is not) ? > > > > > >> These messages are the result of not being able to find the > device, > >> which is consistent with the other things you're seeing. These > are > >> all symptoms of the node device code not running. > > > > OK, I'll wait for your suggestions... > > Do you see messages like: > > > 14:35:01.849: debug : do_open:1205 : node driver 0 Test returned > DECLINED > 14:35:01.849: debug : do_open:1205 : node driver 1 ESX returned > DECLINED > 14:35:01.849: debug : do_open:1205 : node driver 2 remote returned > DECLINED > 14:35:01.849: debug : do_open:1205 : node driver 3 udevDeviceMonitor > returned SUCCESS > > in your debug log? > > > > Frederic. > > > > > >> > >>> Have you any other Idea? > >>> Frederic. > >>> > >>> > >>> ----- "Daniel P. Berrange"<berrange(a)redhat.com> a écrit : > >>> > >>>> On Fri, Feb 12, 2010 at 08:05:21PM +0100, Fr?d?ric Grelot wrote: > >>>>> Hi all, > >>>>> > >>>>> I often update my F12 box with the rawvirt repository, which > >>>>> just > >>>> installed libvirt 0.7.6. Since that, I cannot enumerate physical > >>>> devices (and thus get a beautiful error when I start one of my > vms > >>>> that should use a USB device). > >>>>> > >>>>> I don't think I changed anything else on that box, and, of > >>>>> course, I > >>>> already restarted the libvirt daemon. > >>>>> I haven't restarted the server itself, because it is currently > >>>>> used, > >>>> but I could do it if necessary... > >>>>> > >>>>> I make good use of the qemu hypervisor, and I'm looking forward > >>>>> to > >>>> solve this issue! > >>>>> > >>>>> Frederic. > >>>>> > >>>>> # libvirtd --version libvirtd (libvirt) 0.7.6 > >>>>> > >>>>> # virsh --version 0.7.6 > >>>>> > >>>>> # virsh nodedev-list error :Failed to count node devices error > >>>>> :this function is not supported by the hypervisor: > >>>> virNodeNumOfDevices > >>>> > >>>> It would appear that the udev driver failed to start - there are > >>>> probably messages in syslog about the problem > >>>> > >>>> > >>>> Regards, Daniel -- |: Red Hat, Engineering, London -o- > >>>> http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- > >>>> http://virt-manager.org -o- http://ovirt.org :| |: > >>>> http://autobuild.org -o- http://search.cpan.org/~danberr/ > :| > >>>> |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 > >>>> 7D3B 9505 :| > >>> > >>> -- libvir-list mailing list libvir-list(a)redhat.com > >>> https://www.redhat.com/mailman/listinfo/libvir-list > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list

2 1

Re: [libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot 16 Feb '10

16 Feb '10

The udev driver is actually fully loaded, I even entered "udevadm monitor" in a console, and it reacted as expected when I plugged/unplugged an usb mouse... There is nothing about it in dmesg/boot/message.log, nor libvirt.log (I set up libvirt daemon to log at debug level in /var/log/libvirt/libvirt.log). By the way, I noticed that libvirt generates tons of "error : qemuMonitorTextGetBlockStatsInfo:698 : invalid argument in no stats found for device virtio-diskX" while virt-manager is running (it stops as soon as I stop virt-manager). Is it linked with the bug I saw on the list about libvirt taking over memory after time isn't it? Have you any other Idea? Frederic. ----- "Daniel P. Berrange" <berrange(a)redhat.com> a écrit : > On Fri, Feb 12, 2010 at 08:05:21PM +0100, Fr?d?ric Grelot wrote: > > Hi all, > > > > I often update my F12 box with the rawvirt repository, which just > installed libvirt 0.7.6. Since that, I cannot enumerate physical > devices (and thus get a beautiful error when I start one of my vms > that should use a USB device). > > > > I don't think I changed anything else on that box, and, of course, I > already restarted the libvirt daemon. > > I haven't restarted the server itself, because it is currently used, > but I could do it if necessary... > > > > I make good use of the qemu hypervisor, and I'm looking forward to > solve this issue! > > > > Frederic. > > > > # libvirtd --version > > libvirtd (libvirt) 0.7.6 > > > > # virsh --version > > 0.7.6 > > > > # virsh nodedev-list > > error :Failed to count node devices > > error :this function is not supported by the hypervisor: > virNodeNumOfDevices > > It would appear that the udev driver failed to start - there are > probably > messages in syslog about the problem > > > Regards, > Daniel > -- > |: Red Hat, Engineering, London -o- > http://people.redhat.com/berrange/ :| > |: http://libvirt.org -o- http://virt-manager.org -o- > http://ovirt.org :| > |: http://autobuild.org -o- > http://search.cpan.org/~danberr/ :| > |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B > 9505 :|

2 3

[libvirt] [PATCH] vshCommandParse: placate coverity
by Jim Meyering 16 Feb '10

16 Feb '10

Here are two patches to avoid semblance of a leak. One adds a useless use of VIR_FREE. The other adds an assertion that the pointer to would-be-leaked storage is NULL. There's already an assertion in this file, so no need to add an "#include ...". Take your pick. >From 2070262e7f1a4803e06a6f75bd96091c5754164d Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 14:34:48 +0100 Subject: [PATCH] vshCommandParse: placate coverity There is no real leak here, but Coverity-Prevent thinks there is. It does not see that while there are four ways to return from vshCommandGetToken with VSH_TK_END, none of them results in allocation of a result. * tools/virsh.c (vshCommandParse): Add a (currently) useless VIR_FREE, to ensure that we never leak when vshCommandGetToken returns VSH_TK_END. --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7db48d9..4ffa154 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8222,8 +8222,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) str = end; - if (tk == VSH_TK_END) + if (tk == VSH_TK_END) { + VIR_FREE(tkdata); break; + } if (tk == VSH_TK_ERROR) goto syntaxError; -- 1.7.0.181.g41533 >From 98b0c148d7fd7e11eed7786189db60b485612b51 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 14:34:48 +0100 Subject: [PATCH] vshCommandParse: placate coverity * tools/virsh.c (vshCommandParse): Inform static analyzers that there is no risk of leak when vshCommandGetToken returns VSH_TK_END. --- tools/virsh.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7db48d9..5c45d1c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -8222,8 +8222,10 @@ vshCommandParse(vshControl *ctl, char *cmdstr) str = end; - if (tk == VSH_TK_END) + if (tk == VSH_TK_END) { + assert (tkdata == NULL); break; + } if (tk == VSH_TK_ERROR) goto syntaxError; -- 1.7.0.181.g41533

2 2

[libvirt] [PATCH] qemuMonitorTextAddUSBDisk: avoid unconditional leak
by Jim Meyering 16 Feb '10

16 Feb '10

First the patch, then the (fixed) function: >From d3f0ee1ee11ca5f3709168fdc4e6eb1c56fdfbc0 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 13:51:38 +0100 Subject: [PATCH] qemuMonitorTextAddUSBDisk: avoid unconditional leak * src/qemu/qemu_monitor_text.c (qemuMonitorTextAddUSBDisk): Free command output buffer. --- src/qemu/qemu_monitor_text.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index c71bf1d..62ffcc6 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1229,6 +1229,7 @@ int qemuMonitorTextAddUSBDisk(qemuMonitorPtr mon, cleanup: VIR_FREE(cmd); VIR_FREE(safepath); + VIR_FREE(info); return ret; } -- 1.7.0.181.g41533 int qemuMonitorTextAddUSBDisk(qemuMonitorPtr mon, const char *path) { char *cmd = NULL; char *safepath; int ret = -1; char *info = NULL; safepath = qemuMonitorEscapeArg(path); if (!safepath) { virReportOOMError(); return -1; } if (virAsprintf(&cmd, "usb_add disk:%s", safepath) < 0) { virReportOOMError(); goto cleanup; } if (qemuMonitorCommand(mon, cmd, &info) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to add usb disk")); goto cleanup; } /* If the command failed qemu prints: * Could not add ... */ if (strstr(info, "Could not add ")) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("unable to add USB disk %s: %s"), path, info); goto cleanup; } ret = 0; cleanup: VIR_FREE(cmd); VIR_FREE(safepath); VIR_FREE(info); return ret; }

2 1

[libvirt] [PATCH] tests: avoid NULL deref upon OOM failure
by Jim Meyering 16 Feb '10

16 Feb '10

Pretty obvious: >From 3e79d8839c5051461197a7c484adb32f7f04377a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 09:36:57 +0100 Subject: [PATCH] tests: avoid NULL deref upon OOM failure * tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Handle malloc failure. --- tests/qemuxml2argvtest.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3ee71a3..aa42f99 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -98,7 +98,8 @@ static int testCompareXMLToArgvFiles(const char *xml, len += strlen(*tmp) + 1; tmp++; } - actualargv = malloc(sizeof(*actualargv)*len); + if ((actualargv = malloc(sizeof(*actualargv)*len)) == NULL) + goto fail; actualargv[0] = '\0'; tmp = qenv; while (*tmp) { -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] qemuInitPasswords: avoid unconditional leak
by Jim Meyering 16 Feb '10

16 Feb '10

A successful getVolumeQcowPassphrase allocates "secret". Nothing freed it. >From 2aa6d3c4254f587133e473f4d193ccb023507665 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 09:22:02 +0100 Subject: [PATCH] qemuInitPasswords: avoid unconditional leak * src/qemu/qemu_driver.c (qemuInitPasswords): Free pass-phrase buffer. --- src/qemu/qemu_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03feaf4..e8b7cf6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1733,34 +1733,35 @@ qemuInitPasswords(virConnectPtr conn, for (i = 0 ; i < vm->def->ndisks ; i++) { char *secret; size_t secretLen; if (!vm->def->disks[i]->encryption || !vm->def->disks[i]->src) continue; if (getVolumeQcowPassphrase(conn, vm->def->disks[i], &secret, &secretLen) < 0) goto cleanup; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetDrivePassphrase(priv->mon, vm->def->disks[i]->info.alias, secret); + VIR_FREE(secret); qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) goto cleanup; } } cleanup: return ret; } #define QEMU_PCI_VENDOR_INTEL 0x8086 #define QEMU_PCI_VENDOR_LSI_LOGIC 0x1000 #define QEMU_PCI_VENDOR_REDHAT 0x1af4 #define QEMU_PCI_VENDOR_CIRRUS 0x1013 #define QEMU_PCI_VENDOR_REALTEK 0x10ec #define QEMU_PCI_VENDOR_AMD 0x1022 -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] qemuMonitorTextAddDevice: avoid unconditional leak
by Jim Meyering 16 Feb '10

16 Feb '10

Here's the patch (entire function below): >From 1ee0edab5356ca8cd84a88e4e4a2e4f3cd916f57 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 08:22:34 +0100 Subject: [PATCH] qemuMonitorTextAddDevice: avoid unconditional leak * src/qemu/qemu_monitor_text.c (qemuMonitorTextAddDevice): Free the device name buffer. --- src/qemu/qemu_monitor_text.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index e993699..c71bf1d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2087,6 +2087,7 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, cleanup: VIR_FREE(cmd); VIR_FREE(reply); + VIR_FREE(safedev); return ret; } -- 1.7.0.181.g41533 For reference, here's the entire function: int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr) { char *cmd = NULL; char *reply = NULL; char *safedev; int ret = -1; if (!(safedev = qemuMonitorEscapeArg(devicestr))) { virReportOOMError(); goto cleanup; } if (virAsprintf(&cmd, "device_add %s", safedev) < 0) { virReportOOMError(); goto cleanup; } if (qemuMonitorCommand(mon, cmd, &reply) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("cannot attach %s device"), devicestr); goto cleanup; } if (STRNEQ(reply, "")) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("adding %s device failed: %s"), devicestr, reply); goto cleanup; } ret = 0; cleanup: VIR_FREE(cmd); VIR_FREE(reply); VIR_FREE(safedev); return ret; }

2 1

[libvirt] PATCH [0/3] Volume zeroing
by David Allan 16 Feb '10

16 Feb '10

The following patches implement overwriting a volume with zeros when the volume is deleted. The zeroing happens before the delete, so it works for storage backends that don't support actually deleting volumes as well as the ones that do. The intent is that any future VM assigned that volume will not be able to recover any data belonging to the previous VM. It is not intended to prevent attackers with physical access to the medium from recovering data--it simply writes a single pass of zeros over the medium. If the filesystem containing the volume supports the fiemap ioctl and the volume is a sparse file, the volume zeroing code attempts to use fiemap to locate the mapped extents. It does not attempt to zero a sparse file if it cannot use fiemap. Such an operation could take an essentially unbounded amount of time. Since the volume is being deleted, zeroing has less value in the context of backends that support delete, but does provide value with storage backends that do not zero volumes if they are deleted and recreated. Dave

6 15

[libvirt] [PATCH] Support VCPU hotplug in QEMU guests
by Daniel P. Berrange 16 Feb '10

16 Feb '10

QEMU has a monitor command 'set_cpu' which allows a specific CPU to be toggled between online& offline state. libvirt CPU hotplug does not work in terms of individual indexes CPUs. Thus to support this, we iteratively toggle the online state when the total number of vCPUs is adjusted via libvirt NB, currently untested since QEMU segvs when running this! * src/qemu/qemu_driver.c: Toggle online state for CPUs when doing hotplug * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add monitor API for toggling a CPU's online status via 'set_cpu --- src/qemu/qemu_driver.c | 59 ++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_monitor.c | 14 ++++++++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 44 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 38 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 1 + 7 files changed, 151 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03feaf4..6ef784f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4206,6 +4206,51 @@ cleanup: } +static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + int i, rc; + int ret = -1; + + /* We need different branches here, because we want to offline + * in reverse order to onlining, so any partial fail leaves us in a + * reasonably sensible state */ + if (nvcpus > vm->def->vcpus) { + for (i = vm->def->vcpus ; i < nvcpus ; i++) { + /* Online new CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, 1); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto cleanup; + + vm->def->vcpus++; + } + } else { + for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) { + /* Offline old CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, 0); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto cleanup; + + vm->def->vcpus--; + } + } + + ret = 0; + +cleanup: + return ret; + +unsupported: + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change vcpu count of an active domain")); + goto cleanup; +} + + static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -4228,12 +4273,6 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change vcpu count of an active domain")); - goto endjob; - } - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unknown virt type in domain definition '%d'"), @@ -4254,8 +4293,12 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto endjob; } - vm->def->vcpus = nvcpus; - ret = 0; + if (virDomainObjIsActive(vm)) { + ret = qemudDomainHotplugVcpus(vm, nvcpus); + } else { + vm->def->vcpus = nvcpus; + ret = 0; + } endjob: if (qemuDomainObjEndJob(vm) == 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c1d369b..541d7a1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -912,6 +912,20 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon, return ret; } + +int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online) +{ + int ret; + DEBUG("mon=%p, fd=%d cpu=%d online=%d", mon, mon->fd, cpu, online); + + if (mon->json) + ret = qemuMonitorJSONSetCPU(mon, cpu, online); + else + ret = qemuMonitorTextSetCPU(mon, cpu, online); + return ret; +} + + int qemuMonitorEjectMedia(qemuMonitorPtr mon, const char *devname) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 786ad7a..d560c63 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -144,6 +144,8 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password); int qemuMonitorSetBalloon(qemuMonitorPtr mon, unsigned long newmem); +int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online); + /* XXX should we pass the virDomainDiskDefPtr instead * and hide devname details inside monitor. Reconsider diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c9b8d60..35e529e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -873,6 +873,50 @@ cleanup: } +/* + * Returns: 0 if CPU hotplug not supported, +1 if CPU hotplug worked + * or -1 on failure + */ +int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, + int cpu, int online) +{ + int ret; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("balloon", + "U:cpu", (unsigned long long)cpu, + "s:state", online ? "online" : "offline", + NULL); + virJSONValuePtr reply = NULL; + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) { + /* XXX See if CPU soft-failed due to lack of ACPI */ +#if 0 + if (qemuMonitorJSONHasError(reply, "DeviceNotActive") || + qemuMonitorJSONHasError(reply, "KVMMissingCap")) + goto cleanup; +#endif + + /* See if any other fatal error occurred */ + ret = qemuMonitorJSONCheckError(cmd, reply); + + /* Real success */ + if (ret == 0) + ret = 1; + } + +#if 0 +cleanup: +#endif + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, const char *devname) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 65a70e3..88dfd3c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -57,6 +57,7 @@ int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, const char *password); int qemuMonitorJSONSetBalloon(qemuMonitorPtr mon, unsigned long newmem); +int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, int cpu, int online); int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, const char *devname); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index e993699..1745b20 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -797,6 +797,44 @@ int qemuMonitorTextSetBalloon(qemuMonitorPtr mon, return ret; } + +/* + * Returns: 0 if balloon not supported, +1 if balloon adjust worked + * or -1 on failure + */ +int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, int online) +{ + char *cmd; + char *reply = NULL; + int ret = -1; + + if (virAsprintf(&cmd, "set_cpu %d %s", cpu, online ? "online" : "offline") < 0) { + virReportOOMError(NULL); + return -1; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("could nt change CPU online status")); + VIR_FREE(cmd); + return -1; + } + VIR_FREE(cmd); + + /* If the command failed qemu prints: 'unknown command' + * No message is printed on success it seems */ + if (strstr(reply, "\nunknown command:")) { + /* Don't set error - it is expected CPU onlining fails on many qemu - caller will handle */ + ret = 0; + } else { + ret = 1; + } + + VIR_FREE(reply); + return ret; +} + + int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, const char *devname) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 1937e99..77b8b0f 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -61,6 +61,7 @@ int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, const char *password); int qemuMonitorTextSetBalloon(qemuMonitorPtr mon, unsigned long newmem); +int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, int online); int qemuMonitorTextEjectMedia(qemuMonitorPtr mon, const char *devname); -- 1.6.6

3 4

Re: [libvirt] Live Migration with non-shared storage for kvm
by Kenneth Nagin 16 Feb '10

16 Feb '10

>"Daniel P. Berrange" <berrange(a)redhat.com> wrote on 26/01/2010 16:20:22: > "Daniel P. Berrange" <berrange(a)redhat.com> > 26/01/2010 16:20 > > Please respond to > "Daniel P. Berrange" <berrange(a)redhat.com> > > To > > Kenneth Nagin/Haifa/IBM@IBMIL > > cc > > Subject > > Re: [libvirt] Live Migration with non-shared storage for kvm > > On Tue, Jan 26, 2010 at 04:09:58PM +0200, Kenneth Nagin wrote: > > "Daniel P. Berrange" <berrange(a)redhat.com> wrote on 26/01/2010 12:14:47: > > Since, this my first time submitting something to libvirt I wanted to make > > the initially submission simple, i.e. just adding the flags. > > That sounds fine > > > > > > > > I suggest adding these flags to virDomainMigrate. > > > > > > That sounds reasonable. > > > > > > > If I'm not mistaken qemuMonitorTextMigrate is the function that > > actually > > > > invokes the kvm monitor. > > > > Thus, it would be necessary to pass the flags to > > qemuMonitorTextMigrate.. > > > > But qemuMonitorTextMigrate does not have a flag input parameter. I > > think > > > > the least disruptive way to support the new flags is use the existing > > > > "background" parameter > > > > to pass the flags. Of course this would require some changes to the > > > > upstream functions that are invoked for migration. > > > > > > That is an internal function, so adding extra parameters to it is no > > > problem at all - no need to reuse/abuse existing parameters. > > > > > Are you recommending adding another "flag" parameter or exploiting the > > existing "background" parameter? > > Probably define some flags for the internal API. eg in src/qemu/qemu_monitor.h > add a small enum > > enum { > QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, > QEMU_MONITOR_MIGRATE_STORAGE = 1 << 1 > ...etc > }; > > and then just replace the existing background parameter with a flags > parameter > > > Daniel > -- > |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| > |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| Sorry about the delayed response. I had some pressing work items and family issues that took precedence. In order to support kvm's live migration with non-shared storage I added two migration flags that user can invoke: VIR_MIGRATE_NON_SHARED_DISK = (1 << 6), /* migration with non-shared storage with full disk copy */ VIR_MIGRATE_NON_SHARED_INC = (1 << 7), /* migration with non-shared storage with incremental copy */ /* (same base image shared between source and destination) */ Likewise I add complementary flags to virsh migrate: non_shared_disk and non_shared_inc As you suggested I also added internal flags to be passed in the "background" parameter to the qemu monitor: typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with non-shared storage with incremental copy */ QEMU_MONITOR_MIGRATION_FLAGS_LAST }; I updated qemu_driver.c's doNativeMigrate and doTunnelMigrate to map the external flags to the internal flags. I updated qemu_monitor_text's qemuMonitorTextMigrate to map the internal flags to the qemu monitor's command line flags. I tested the doNativeMigrate usage of the two flags. However, I could not test the doTunnelMigrate usage because I wasn't sure how it works. Also, qemudDomainSave ends up calling qemuMonitorTextMigrate, but it didn't support the external flags so I assumed that it was not relevant. Here is the diff patch file from the current git: (See attached file: migration_with_non_shared.patch) - Kenneth Nagin

1 0

[libvirt] [PATCH 0/5] macvtap support for Qemu/KVM VMs via libvirt
by Stefan Berger 16 Feb '10

16 Feb '10

Hello! This is a re-post of previously posted patches following Daniel Berrange's request for changes along with other fixes PLUS a rebase to the latest code where the conn parameter is missing along with changes requested by Daniel Veillard. The following patches provide support for making the macvtap networking device type available to Qemu/KVM VMs. The patches rely on the macvtap driver that just became available through the Linux net-next tree (fixes still may be necessary) and make the device available to Qemu/KVM via a tap file descriptor similar to a 'regular' tap device. Following up on previous discussions, the libvirt patches allow using the following XML in the domain description to enable qemu network connectivity via this type of device: <interface type='direct'> <source dev='eth1' mode='vepa'/> <model type='virtio'/> </interface> The above XML indicates that eth1 is the Ethernet interface to link the macvtap device to and communicate to the network. As a consequence, libvirt will create an instance of a macvtap device, assign it the same MAC address as the VM's interface has and open a file descriptor of the associated character device /dev/tap%d and pass it via command line to Qemu/kvm. In the above XML the mode can be chosen as 'vepa', 'private' or 'bridge' and is by default set to 'vepa'(by the driver) if omitted. Attachment and detachment of macvtap to/from a running VM also works. Regards, Stefan

3 4

[libvirt] [PATCH] libvirt-override.c: avoid a leak upon call with invalid argument
by Jim Meyering 16 Feb '10

16 Feb '10

>From 49d7a268574772d362dc374b531ff80ec87f5062 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 16 Feb 2010 08:07:38 +0100 Subject: [PATCH] libvirt-override.c: avoid a leak upon call with invalid argument * python/libvirt-override.c (libvirt_virConnectBaselineCPU): Don't leak the xmlcpus buffer upon encountering a non-string list element. --- python/libvirt-override.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index a71766a..2447ad7 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1,16 +1,16 @@ /* * libvir.c: this modules implements the main part of the glue of the * libvir library and the Python interpreter. It provides the * entry points where an automatically generated stub is * unpractical * - * Copyright (C) 2005, 2007-2009 Red Hat, Inc. + * Copyright (C) 2005, 2007-2010 Red Hat, Inc. * * Daniel Veillard <veillard(a)redhat.com> */ #include <config.h> /* Horrible kludge to work around even more horrible name-space pollution via Python.h. That file includes /usr/include/python2.5/pyconfig*.h, which has over 180 autoconf-style HAVE_* definitions. Shame on them. */ @@ -2040,20 +2040,22 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, if (PyList_Check(list)) { int i; ncpus = PyList_Size(list); if ((xmlcpus = malloc(ncpus * sizeof(*xmlcpus))) == NULL) return VIR_PY_INT_FAIL; for (i = 0; i < ncpus; i++) { xmlcpus[i] = PyString_AsString(PyList_GetItem(list, i)); - if (xmlcpus[i] == NULL) + if (xmlcpus[i] == NULL) { + free(xmlcpus); return VIR_PY_INT_FAIL; + } } } LIBVIRT_BEGIN_ALLOW_THREADS; base_cpu = virConnectBaselineCPU(conn, xmlcpus, ncpus, flags); LIBVIRT_END_ALLOW_THREADS; free(xmlcpus); -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH 4/4] remote: Detect 'nc' version incompatibilities
by Cole Robinson 16 Feb '10

16 Feb '10

This ugly thing is a shell script to detect availability of the -q option for 'nc': debian and suse based distros need this flag to ensure the remote nc will exit on EOF, so it will go away when we close the tunnel. If it doesn't go away, a useless 'nc' process is left sitting on the remote host. Fedora's 'nc' doesn't have this option, so we can't blindly pass -q. More info here: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=564034 We also detect the -U option, since some nc versions don't support it, which has caused quite some user confusion. Connecting to a box and using the incorrect nc: virsh --connect qemu+ssh://root@debhost/system?netcat=/bin/nc.traditional error: server closed connection: /bin/nc.traditional: invalid option -- U nc -h for help openbsd nc is required error: failed to connect to the hypervisor Test with Fedora 12, RHEL 5.4, and Debian Lenny --- src/remote/remote_driver.c | 52 ++++++++++++++++++++++++++++++++++++++------ 1 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7f92fd0..c3258d3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -732,8 +732,49 @@ doRemoteOpen (virConnectPtr conn, } case trans_ssh: { - int j, nr_args = 6; + int j, nr_args = 0; + char *nc_command; + + const char *nc_bin = netcat ? netcat : "nc"; + const char *sock_path = (sockname ? sockname : + (flags & VIR_CONNECT_RO + ? LIBVIRTD_PRIV_UNIX_SOCKET_RO + : LIBVIRTD_PRIV_UNIX_SOCKET)); + + /* + * Build 'nc' command run on the remote host + * + * This ugly thing is a shell script to detect availability of + * the -q option for 'nc': debian and suse based distros need this + * flag to ensure the remote nc will exit on EOF, so it will go away + * when we close the tunnel. If it doesn't go away, a useless 'nc' + * process is left sitting on the remote host. + * + * Fedora's 'nc' doesn't have this option, and apparently defaults + * to the desired behavior. + */ + const char *nc_detect_template = ( + "NCOUT=$(%s -U 2>&1);" + "echo \"$NCOUT\" | grep -q 'invalid option';" + "if [ $? -eq 0 ] ; then" + " echo \"$NCOUT\" >&2;" + " echo openbsd 'nc' is required >&2;" + " exit 1;" + "fi;" + "" + "%s -q 2>&1 | grep -q 'requires an argument';" + "if [ $? -eq 0 ] ; then" + " CMD='-q 0';" + "else" + " CMD='';" + "fi;" + "%s $CMD -U %s;"); + + if (virAsprintf(&nc_command, nc_detect_template, + nc_bin, nc_bin, nc_bin, sock_path) < 0) + goto out_of_memory; + nr_args += 4; /* ssh $hostname $netcat_command NULL*/ if (username) nr_args += 2; /* For -l username */ if (no_tty) nr_args += 5; /* For -T -o BatchMode=yes -e none */ if (port) nr_args += 2; /* For -p port */ @@ -765,12 +806,9 @@ doRemoteOpen (virConnectPtr conn, cmd_argv[j++] = strdup ("none"); } cmd_argv[j++] = strdup (priv->hostname); - cmd_argv[j++] = strdup (netcat ? netcat : "nc"); - cmd_argv[j++] = strdup ("-U"); - cmd_argv[j++] = strdup (sockname ? sockname : - (flags & VIR_CONNECT_RO - ? LIBVIRTD_PRIV_UNIX_SOCKET_RO - : LIBVIRTD_PRIV_UNIX_SOCKET)); + + cmd_argv[j++] = nc_command; + cmd_argv[j++] = 0; assert (j == nr_args); for (j = 0; j < (nr_args-1); j++) -- 1.6.5.2

3 7

[libvirt] [PATCH] vboxDomainDumpXML: avoid a leak on OOM error path
by Jim Meyering 15 Feb '10

15 Feb '10

Yet another... >From 05e87caf9da26f9038935d7e54b6038b9362875b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 20:09:55 +0100 Subject: [PATCH] vboxDomainDumpXML: avoid a leak on OOM error path * src/vbox/vbox_tmpl.c (vboxDomainDumpXML): Free vboxCallback buffer upon OOM. --- src/vbox/vbox_tmpl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index d1a701e..8a9af52 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5276,10 +5276,11 @@ static nsresult vboxCallbackQueryInterface(nsISupports *pThis, const nsID *iid, static IVirtualBoxCallback *vboxAllocCallbackObj(void) { IVirtualBoxCallback *vboxCallback = NULL; - /* Allocate, Initialize and return a validi + /* Allocate, Initialize and return a valid * IVirtualBoxCallback object here */ if ((VIR_ALLOC(vboxCallback) < 0) || (VIR_ALLOC(vboxCallback->vtbl) < 0)) { + VIR_FREE(vboxCallback); virReportOOMError(); return NULL; } -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] virNodeDevCapScsiHostParseXML: avoid an unconditional leak
by Jim Meyering 15 Feb '10

15 Feb '10

It looks like there may still be error-path leaks (the buffers pointed to by nodes[i]), but that can wait for another day. >From c293a97ed55c99c87bc658aaffc10831f8ba8c68 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 19:54:45 +0100 Subject: [PATCH] virNodeDevCapScsiHostParseXML: avoid an unconditional leak * src/conf/node_device_conf.c (virNodeDevCapScsiHostParseXML): Free the "nodes" buffer allocated by virXPathNodeSet. --- src/conf/node_device_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4b65953..09c0f41 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -797,6 +797,7 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, out: VIR_FREE(type); ctxt->node = orignode; + VIR_FREE(nodes); return ret; } -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] uml_driver.c: avoid leak upon failure
by Jim Meyering 15 Feb '10

15 Feb '10

Without this, it looked like every caller (there's only one; the other use is if-0'd out) could leak the result buffer. >From 8b8874b956c822562151cb90b1a8950d17b565ac Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 19:22:38 +0100 Subject: [PATCH] uml_driver.c: avoid leak upon failure * src/uml/uml_driver.c (umlMonitorCommand): This function would sometimes return -1, yet fail to free the "reply" it had allocated. Hence, no caller would know to free the corresponding argument. When returning -1, be sure to free all allocated resources. --- src/uml/uml_driver.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 5049c92..10268db 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1,7 +1,7 @@ /* * uml_driver.c: core driver methods for managing UML guests * - * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -757,7 +757,10 @@ static int umlMonitorCommand(virConnectPtr conn, VIR_DEBUG("Command reply is '%s'", NULLSTR(retdata)); - *reply = retdata; + if (ret < 0) + VIR_FREE(retdata); + else + *reply = retdata; return ret; -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH 2/4] qemu: Make SetVcpu command hotplug only
by Cole Robinson 15 Feb '10

15 Feb '10

Similar to the Set*Mem commands, this implementation was bogus and misleading. Make it clear this is a hotplug only operation, and that the hotplug piece isn't even implemented. Also drop the overkill maxvcpus validation: we don't perform this check at XML define time so clearly no one is missing it, and there is always the risk that our info will be out of date, possibly preventing legitimate CPU values. --- src/qemu/qemu_driver.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56a450c..ef1f638 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4100,12 +4100,11 @@ cleanup: } -static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { +static int qemudDomainSetVcpus(virDomainPtr dom, + ATTRIBUTE_UNUSED unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - int max; int ret = -1; - const char *type; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4119,41 +4118,14 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); goto cleanup; - - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change vcpu count of an active domain")); - goto endjob; - } - - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto endjob; } - if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not determine max vcpus for the domain")); - goto endjob; - } - - if (nvcpus > max) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the domain: %d > %d"), nvcpus, max); - goto endjob; - } - - vm->def->vcpus = nvcpus; - ret = 0; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cpu hotplug not yet supported")); cleanup: if (vm) -- 1.6.5.2

2 2

[libvirt] [PATCH] openvz_driver.c: avoid leak on OOM error path
by Jim Meyering 15 Feb '10

15 Feb '10

It looks like openvzFreeDriver cannot call VIR_FREE on its buffer argument, because of the use in the function that just happens to be a few lines below. These are the only two uses of openvzFreeDriver. >From 0a616cd03cca9c9d89f23dadd52cd23c30789aca Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 18:33:38 +0100 Subject: [PATCH] openvz_driver.c: avoid leak on OOM error path * src/openvz/openvz_driver.c (openvzOpen): Free "driver" buffer upon failure. --- src/openvz/openvz_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 68d0398..3a8a82f 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1245,22 +1245,23 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn, goto cleanup; if (openvzExtractVersion(conn, driver) < 0) goto cleanup; conn->privateData = driver; return VIR_DRV_OPEN_SUCCESS; cleanup: openvzFreeDriver(driver); + VIR_FREE(driver); return VIR_DRV_OPEN_ERROR; }; static int openvzClose(virConnectPtr conn) { struct openvz_driver *driver = conn->privateData; openvzFreeDriver(driver); conn->privateData = NULL; return 0; } -- 1.7.0.181.g41533

2 3

[libvirt] [PATCH 3/5] [FIX] macvtap support for libvirt -- qemu support
by Stefan Berger 15 Feb '10

15 Feb '10

I needed to fix the case where macvtap support is not to be compiled in. This part adds support for qemu making a macvtap tap device available via file descriptor passed to qemu command line. This also attempts to tear down the macvtap device when a VM terminates. This includes support for attachment and detachment to/from running VM. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

3 3

[libvirt] [PATCH] Treat missing QEMU 'thread_id' as non-fatal in JSON monitor
by Daniel P. Berrange 15 Feb '10

15 Feb '10

The plain QEMU tree does not include 'thread_id' in the JSON output. Thus we need to treat it as non-fatal if missing. * src/qemu/qemu_monitor_json.c: Treat missing thread_id as non-fatal --- src/qemu/qemu_monitor_json.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9875f38..49e0370 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -630,8 +630,9 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, } if (virJSONValueObjectGetNumberInt(entry, "thread_id", &thread) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cpu information was missing thread ID")); + /* Only qemu-kvm tree includs thread_id, so treat this as + non-fatal, simply returning no data */ + ret = 0; goto cleanup; } -- 1.6.6

2 1

[libvirt] [PATCH] Make error reporting for QEMU JSON mode more friendly
by Daniel P. Berrange 15 Feb '10

15 Feb '10

Current error reporting for JSON mode returns the full JSON command string and full JSON error string. This is not very user friendly, so this change makes the error report only contain the basic command name, and friendly error message description string. The full JSON data is logged instead. * src/qemu/qemu_monitor_json.c: Always return the 'desc' field from the JSON error message to users. --- src/qemu/qemu_monitor_json.c | 58 ++++++++++++++++++++++++++++------------- 1 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f0dcf81..9875f38 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -246,15 +246,33 @@ qemuMonitorJSONCommand(qemuMonitorPtr mon, * * XXX see qerror.h for different klasses & fill out useful params */ -static char *qemuMonitorJSONStringifyError(virJSONValuePtr error) +static const char * +qemuMonitorJSONStringifyError(virJSONValuePtr error) { const char *klass = virJSONValueObjectGetString(error, "class"); + const char *detail = NULL; - if (klass) { - return strdup(klass); - } else { - return strdup(_("Missing QEMU error klass")); - } + /* The QMP 'desc' field is usually sufficient for our generic + * error reporting needs. + */ + if (klass) + detail = virJSONValueObjectGetString(error, "desc"); + + + if (!detail) + detail = "unknown QEMU command error"; + + return detail; +} + +static const char * +qemuMonitorJSONCommandName(virJSONValuePtr cmd) +{ + const char *name = virJSONValueObjectGetString(cmd, "execute"); + if (name) + return name; + else + return "<unknown>"; } static int @@ -266,20 +284,21 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd, char *cmdstr = virJSONValueToString(cmd); char *replystr = virJSONValueToString(reply); - if (!error) { - VIR_DEBUG("Saw a JSON error, but value is null for %s: %s", - cmdstr, replystr); + /* Log the full JSON formatted command & error */ + VIR_DEBUG("unable to execute QEMU command %s: %s", + cmdstr, replystr); + + /* Only send the user the command name + friendly error */ + if (!error) qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': '%s'"), - cmdstr, replystr); - } else { - VIR_DEBUG("Got a JSON error set for %s", cmdstr); - char *detail = qemuMonitorJSONStringifyError(error); + _("unable to execute QEMU command '%s'"), + qemuMonitorJSONCommandName(cmd)); + else qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': %s ('%s')"), - cmdstr, detail, replystr); - VIR_FREE(detail); - } + _("unable to execute QEMU command '%s': %s"), + qemuMonitorJSONCommandName(cmd), + qemuMonitorJSONStringifyError(error)); + VIR_FREE(cmdstr); VIR_FREE(replystr); return -1; @@ -290,7 +309,8 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd, VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", cmdstr, replystr); qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': '%s'"), cmdstr, replystr); + _("unable to execute QEMU command '%s'"), + qemuMonitorJSONCommandName(cmd)); VIR_FREE(cmdstr); VIR_FREE(replystr); return -1; -- 1.6.6

2 1

[libvirt] [PATCH] Fix check for primary IDE controller in QEMU PCI slot assignment
by Daniel P. Berrange 15 Feb '10

15 Feb '10

A typo in the check for the primary IDE controller could cause a crash on restore depending on the exact guest config. * src/qemu/qemu_conf.c: Fix s/video/controller/ typo & slot number typo --- src/qemu/qemu_conf.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 00f761d..dd5efd6 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2085,10 +2085,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && def->controllers[i]->idx == 0) { if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (def->videos[i]->info.addr.pci.domain != 0 || - def->videos[i]->info.addr.pci.bus != 0 || - def->videos[i]->info.addr.pci.slot != 2 || - def->videos[i]->info.addr.pci.function != 0) { + if (def->controllers[i]->info.addr.pci.domain != 0 || + def->controllers[i]->info.addr.pci.bus != 0 || + def->controllers[i]->info.addr.pci.slot != 1 || + def->controllers[i]->info.addr.pci.function != 1) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary IDE controller must have PCI address 0:0:1.1")); goto error; -- 1.6.6

2 1

[libvirt] [PATCH] interface: Use proper return codes in the open function
by Matthias Bolte 15 Feb '10

15 Feb '10

The open function returned -1 in case of an error, but -1 maps to VIR_DRV_OPEN_DECLINED instead of VIR_DRV_OPEN_ERROR. --- src/interface/netcf_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 2753049..7f4d43d 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -139,7 +139,7 @@ static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn, } conn->interfacePrivateData = driverState; - return 0; + return VIR_DRV_OPEN_SUCCESS; netcf_error: if (driverState->netcf) @@ -150,7 +150,7 @@ netcf_error: mutex_error: VIR_FREE(driverState); alloc_error: - return -1; + return VIR_DRV_OPEN_ERROR; } static int interfaceCloseInterface(virConnectPtr conn) -- 1.6.3.3

2 2

Re: [libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot 15 Feb '10

15 Feb '10

> > # libvirtd --version > > libvirtd (libvirt) 0.7.6 > > > > # virsh --version > > 0.7.6 > > > > # virsh nodedev-list > > error :Failed to count node devices > > error :this function is not supported by the hypervisor: > virNodeNumOfDevices I restarted the server, with no change. I uninstalled and reinstalled libvirt, same result. I had a look at libvirt logs, there is nothing special. I tried several other things, with no luck. I check that "lshal" gives result : it is actually the case... > It would appear that the udev driver failed to start - there are > probably > messages in syslog about the problem I'll check that. However I don't know what happened with this last update, physical devices worked great before it... If what you say is right, it may be linked with some other update that occured at the same time... Thanks for you help! > > Regards, > Daniel > -- > |: Red Hat, Engineering, London -o- > http://people.redhat.com/berrange/ :| > |: http://libvirt.org -o- http://virt-manager.org -o- > http://ovirt.org :| > |: http://autobuild.org -o- > http://search.cpan.org/~danberr/ :| > |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B > 9505 :|

1 0

[libvirt] Cannot enumerate physical devices with libvirt 0.7.6
by Frédéric Grelot 15 Feb '10

15 Feb '10

Hi all, I often update my F12 box with the rawvirt repository, which just installed libvirt 0.7.6. Since that, I cannot enumerate physical devices (and thus get a beautiful error when I start one of my vms that should use a USB device). I don't think I changed anything else on that box, and, of course, I already restarted the libvirt daemon. I haven't restarted the server itself, because it is currently used, but I could do it if necessary... I make good use of the qemu hypervisor, and I'm looking forward to solve this issue! Frederic. # libvirtd --version libvirtd (libvirt) 0.7.6 # virsh --version 0.7.6 # virsh nodedev-list error :Failed to count node devices error :this function is not supported by the hypervisor: virNodeNumOfDevices

2 1

[libvirt] [PATCH] Convert virSecurityReportError into a macro
by Matthias Bolte 15 Feb '10

15 Feb '10

--- src/security/security_driver.c | 18 ------------------ src/security/security_driver.h | 6 +++--- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/security/security_driver.c b/src/security/security_driver.c index 27945a6..4c98190 100644 --- a/src/security/security_driver.c +++ b/src/security/security_driver.c @@ -90,24 +90,6 @@ virSecurityDriverStartup(virSecurityDriverPtr *drv, return -2; } -void -virSecurityReportError(int code, const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage) - 1, fmt, args); - va_end(args); - } else - errorMessage[0] = '\0'; - - virRaiseError(NULL, NULL, NULL, VIR_FROM_SECURITY, code, - VIR_ERR_ERROR, NULL, NULL, NULL, -1, -1, "%s", - errorMessage); -} - /* * Helpers */ diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 8860d81..15671b3 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -88,9 +88,9 @@ int virSecurityDriverStartup(virSecurityDriverPtr *drv, int virSecurityDriverVerify(virDomainDefPtr def); -void -virSecurityReportError(int code, const char *fmt, ...) - ATTRIBUTE_FMT_PRINTF(2, 3); +#define virSecurityReportError(code, fmt...) \ + virReportErrorHelper(NULL, VIR_FROM_SECURITY, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /* Helpers */ void virSecurityDriverInit(virSecurityDriverPtr drv); -- 1.6.3.3

2 2

[libvirt] How to shut down every VM when libvirt is stopped
by Laurent Léonard 13 Feb '10

13 Feb '10

Hi, With libvirt 0.4.6 for example, the behaviour when we stopped libvirt was to shut down every VM. That was a problem in many cases (upgrades, etc.) but that behaviour was very useful with high avalability systems like Heartbeat. With an Hearbeat + DRBD system, if the libvirt service is stopped by Heartbeat, every VM should be stopped too. Otherwise we will experience problems and disk corruption. Is there any way to configure libvirt to have that old behaviour ? Or do I have to shut down/kill every VM in the stop target in the init script ? Thank you, -- Laurent Léonard

1 0

[libvirt] [PATCH] Add persistence of PCI addresses to QEMU
by Daniel P. Berrange 12 Feb '10

12 Feb '10

Current PCI addresses are allocated at time of VM startup. To make them truely persistent, it is neccessary to do this at time of virDomainDefine/virDomainCreate. The code in qemuStartVMDaemon still remains in order to cope with upgrades from older libvirt releases * src/qemu/qemu_driver.c: Rename existing qemuAssignPCIAddresses to qemuDetectPCIAddresses. Add new qemuAssignPCIAddresses which does auto-allocation upfront. Call qemuAssignPCIAddresses from qemuDomainDefine and qemuDomainCreate to assign PCI addresses that can then be persisted. Don't clear PCI addresses at shutdown if they are intended to be persistent --- src/qemu/qemu_driver.c | 84 +++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03d0f5f..69187fc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -95,6 +95,7 @@ struct _qemuDomainObjPrivate { int *vcpupids; qemuDomainPCIAddressSetPtr pciaddrs; + int persistentAddrs; }; static int qemudShutdown(void); @@ -857,6 +858,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq virDomainObjPtr obj = payload; struct qemud_driver *driver = opaque; qemuDomainObjPrivatePtr priv; + unsigned long long qemuCmdFlags; virDomainObjLock(obj); @@ -872,6 +874,15 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq goto error; } + /* XXX we should be persisting the original flags in the XML + * not re-detecting them, since the binary may have changed + * since launch time */ + if (qemudExtractVersionInfo(obj->def->emulator, + NULL, + &qemuCmdFlags) >= 0 && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) + priv->persistentAddrs = 1; + if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(obj->def))) goto error; @@ -1976,7 +1987,7 @@ qemuGetPCIWatchdogVendorProduct(virDomainWatchdogDefPtr def, * some static addrs on CLI. Have to check that... */ static int -qemuAssignPCIAddresses(virDomainObjPtr vm, +qemuDetectPCIAddresses(virDomainObjPtr vm, qemuMonitorPCIAddress *addrs, int naddrs) { @@ -2098,7 +2109,7 @@ qemuInitPCIAddresses(struct qemud_driver *driver, &addrs); qemuDomainObjExitMonitorWithDriver(driver, vm); - ret = qemuAssignPCIAddresses(vm, addrs, naddrs); + ret = qemuDetectPCIAddresses(vm, addrs, naddrs); VIR_FREE(addrs); @@ -2141,6 +2152,44 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) { return -1; } + +static int +qemuAssignPCIAddresses(virDomainDefPtr def) +{ + int ret = -1; + unsigned long long qemuCmdFlags = 0; + qemuDomainPCIAddressSetPtr addrs = NULL; + struct stat sb; + + if (stat(def->emulator, &sb) < 0) { + virReportSystemError(errno, + _("Cannot find QEMU binary %s"), + def->emulator); + goto cleanup; + } + + if (qemudExtractVersionInfo(def->emulator, + NULL, + &qemuCmdFlags) < 0) + goto cleanup; + + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (!(addrs = qemuDomainPCIAddressSetCreate(def))) + goto cleanup; + + if (qemuAssignDevicePCISlots(def, addrs) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + qemuDomainPCIAddressSetFree(addrs); + + return ret; +} + + static pciDeviceList * qemuGetPciHostDeviceList(virDomainDefPtr def) { @@ -2662,7 +2711,15 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; } + /* + * Normally PCI addresses are assigned inhe virDomainCreate + * or virDomainDefine methods. We might still need to assign + * some here to cope with the question of upgrades. Regardless + * we also need to populate the PCi address set cache for later + * use in hotplug + */ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + /* Populate cache with current addresses */ if (priv->pciaddrs) { qemuDomainPCIAddressSetFree(priv->pciaddrs); priv->pciaddrs = NULL; @@ -2670,8 +2727,14 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (!(priv->pciaddrs = qemuDomainPCIAddressSetCreate(vm->def))) goto cleanup; + + /* Assign any remaining addresses */ if (qemuAssignDevicePCISlots(vm->def, priv->pciaddrs) < 0) goto cleanup; + + priv->persistentAddrs = 1; + } else { + priv->persistentAddrs = 0; } vm->def->id = driver->nextvmid++; @@ -2903,10 +2966,12 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, VIR_FREE(vm->def->seclabel.imagelabel); } - virDomainDefClearPCIAddresses(vm->def); virDomainDefClearDeviceAliases(vm->def); - qemuDomainPCIAddressSetFree(priv->pciaddrs); - priv->pciaddrs = NULL; + if (!priv->persistentAddrs) { + virDomainDefClearPCIAddresses(vm->def); + qemuDomainPCIAddressSetFree(priv->pciaddrs); + priv->pciaddrs = NULL; + } qemuDomainReAttachHostDevices(driver, vm->def); @@ -3352,6 +3417,12 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; + if (qemudCanonicalizeMachine(driver, def) < 0) + goto cleanup; + + if (qemuAssignPCIAddresses(def) < 0) + goto cleanup; + if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def))) @@ -5049,6 +5120,9 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; + if (qemuAssignPCIAddresses(def) < 0) + goto cleanup; + if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, def))) { -- 1.6.5.2

2 3

[libvirt] [PATCH] Fix compliation of AppArmor related code
by Matthias Bolte 12 Feb '10

12 Feb '10

Broken by the latest commits to remove the virConnectPtr parameter from internal functions. I just pushed this one, as it is an obvious fix. --- src/security/security_apparmor.c | 4 ++-- src/security/virt-aa-helper.c | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 23f40f8..db04d5c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -262,7 +262,7 @@ use_apparmor(void) char *libvirt_daemon = NULL; if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) { - virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR, + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not find libvirtd")); return rc; } @@ -295,7 +295,7 @@ AppArmorSecurityDriverProbe(void) } if (!virFileExists(template)) { - virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR, + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("template \'%s\' does not exist"), template); goto clean; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 619c8c3..066e18b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -690,7 +690,7 @@ get_definition(vahControl * ctl, const char *xmlStr) goto exit; } - ctl->def = virDomainDefParseString(NULL, ctl->caps, xmlStr, 0); + ctl->def = virDomainDefParseString(ctl->caps, xmlStr, 0); if (ctl->def == NULL) { vah_error(ctl, 0, "could not parse XML"); goto exit; @@ -767,8 +767,7 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) } static int -file_iterate_cb(virConnectPtr conn ATTRIBUTE_UNUSED, - usbDevice *dev ATTRIBUTE_UNUSED, +file_iterate_cb(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { virBufferPtr buf = opaque; @@ -844,8 +843,7 @@ get_files(vahControl * ctl) if (usb == NULL) continue; - rc = usbDeviceFileIterate(NULL, usb, - file_iterate_cb, &buf); + rc = usbDeviceFileIterate(usb, file_iterate_cb, &buf); usbFreeDevice(usb); if (rc != 0) goto clean; -- 1.6.3.3

2 1

[libvirt] dname parameter ignored in qemudDomainMigratePrepareTunnel{, 2}
by Jim Meyering 11 Feb '10

11 Feb '10

clang warned about a dead-store in src/qemu/qemu_driver.c, since dname is never used thereafter: http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_driver.c;h=0d7… /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name; That stmt was initially added with a following use, if (!vm) vm = virDomainFindByName(&driver->domains, dname); But the use was removed by this commit, rendering the store "dead", and the dname parameter effectively unused: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=c26cb9234f4b9fa46d7caa… The same thing happened in both qemudDomainMigratePrepareTunnel and qemudDomainMigratePrepareTunnel2 Shouldn't they be doing something like this instead? if (dname) def->name = dname; But def->name is already allocated. Better free it first. And is dname "known" to be allocated from the heap? That's not clear from the little documentation I've read so far, so I've strdup'd it below: Here's a possible patch: >From b51a39aed62ee5c8db733cecfe6eef0c34a7f989 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Feb 2010 16:46:21 +0100 Subject: [PATCH] qemu_driver.c: honor dname parameter once again Since c26cb9234f4b9fa46d7caa3385ae36704167c53f, the dname parameter has been ignored by these two functions. Use it. * src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Honor dname parameter once again. (qemudDomainMigratePrepare2): Likewise. --- src/qemu/qemu_driver.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d77d57..9ff712c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7434,7 +7434,12 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, } /* Target domain name, maybe renamed. */ - dname = dname ? dname : def->name; + if (dname) { + VIR_FREE(def->name); + def->name = strdup(dname); + if (def->name == NULL) + goto cleanup; + } if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; @@ -7660,7 +7665,12 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, } /* Target domain name, maybe renamed. */ - dname = dname ? dname : def->name; + if (dname) { + VIR_FREE(def->name); + def->name = strdup(dname); + if (def->name == NULL) + goto cleanup; + } if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; -- 1.7.0.rc2.170.gbc565

2 2

[libvirt] [PATCH] Mark all error messages for translation
by Jiri Denemark 11 Feb '10

11 Feb '10

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/conf/storage_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eb1c9d9..2352637 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1464,7 +1464,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, if (!virFileMatchesNameSuffix(file, def->name, ".xml")) { virStorageReportError(VIR_ERR_INVALID_STORAGE_POOL, - "Storage pool config filename '%s' does not match pool name '%s'", + _("Storage pool config filename '%s' does not match pool name '%s'"), path, def->name); virStoragePoolDefFree(def); return NULL; @@ -1524,7 +1524,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, if (virFileBuildPath(configDir, entry->d_name, NULL, path, PATH_MAX) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "Config filename '%s/%s' is too long", + _("Config filename '%s/%s' is too long"), configDir, entry->d_name); continue; } @@ -1532,7 +1532,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, if (virFileBuildPath(autostartDir, entry->d_name, NULL, autostartLink, PATH_MAX) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "Autostart link path '%s/%s' is too long", + _("Autostart link path '%s/%s' is too long"), autostartDir, entry->d_name); continue; } -- 1.6.6.1

2 2

[libvirt] [PATCH] remote_driver.c: avoid leak on OOM error path
by Jim Meyering 11 Feb '10

11 Feb '10

Any "goto out_of_memory" taken between the vars = qparam_query_parse (query) allocation and the free_qparam_set call would result in a leak. Here's the fix: >From a3a21142e40962c960eae882ac9edcc3bd17cb8b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Feb 2010 12:51:31 +0100 Subject: [PATCH] remote_driver.c: avoid leak on OOM error path * src/remote/remote_driver.c (doRemoteOpen): Don't leak a qparam_set buffer upon OOM error. --- src/remote/remote_driver.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e17db3d..cc543fc 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2,7 +2,7 @@ * remote_internal.c: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -370,6 +370,7 @@ doRemoteOpen (virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags) { + struct qparam_set *vars = NULL; int wakeupFD[2] = { -1, -1 }; char *transport_str = NULL; enum { @@ -458,7 +459,6 @@ doRemoteOpen (virConnectPtr conn, * feasibly it might contain variables needed by the real driver, * although that won't be the case for now). */ - struct qparam_set *vars; struct qparam *var; int i; char *query; @@ -551,6 +551,7 @@ doRemoteOpen (virConnectPtr conn, } free_qparam_set (vars); + vars = NULL; } else { /* Probe URI server side */ name = strdup(""); @@ -933,6 +934,8 @@ doRemoteOpen (virConnectPtr conn, out_of_memory: virReportOOMError(); + if (vars) + free_qparam_set (vars); failed: /* Close the socket if we failed. */ -- 1.7.0.rc2.170.gbc565

2 2

[libvirt] [PATCH] Swap position of nmodels and models parameters in cpuDecode()
by Jiri Denemark 11 Feb '10

11 Feb '10

All other libvirt functions use array first and then number of elements in that array. Let's make cpuDecode follow this rule. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/cpu/cpu.c | 6 +++--- src/cpu/cpu.h | 8 ++++---- src/cpu/cpu_x86.c | 4 ++-- src/qemu/qemu_conf.c | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index ae8d37d..e9ecc98 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -119,8 +119,8 @@ cpuCompare(virCPUDefPtr host, int cpuDecode(virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models) + const char **models, + unsigned int nmodels) { struct cpuArchDriver *driver; @@ -146,7 +146,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, nmodels, models); + return driver->decode(cpu, data, models, nmodels); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 1e04993..7ee0ce6 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -48,8 +48,8 @@ typedef virCPUCompareResult typedef int (*cpuArchDecode) (virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models); + const char **models, + unsigned int nmodels); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -95,8 +95,8 @@ cpuCompare (virCPUDefPtr host, extern int cpuDecode (virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models); + const char **models, + unsigned int nmodels); extern int cpuEncode (const char *arch, diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 7ae7fbf..c6ed078 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -940,8 +940,8 @@ x86GuestData(virCPUDefPtr host, static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models) + const char **models, + unsigned int nmodels) { int ret = -1; struct x86_map *map; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3988582..ca32dc0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1016,7 +1016,7 @@ qemudCapsInitCPU(virCapsPtr caps, cpu->threads = nodeinfo.threads; if (!(data = cpuNodeData(arch)) - || cpuDecode(cpu, data, 0, NULL) < 0) + || cpuDecode(cpu, data, NULL, 0) < 0) goto error; caps->host.cpu = cpu; @@ -2959,7 +2959,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (VIR_ALLOC(guest) < 0 || !(guest->arch = strdup(ut->machine))) goto no_memory; - if (cpuDecode(guest, data, ncpus, cpus) < 0) + if (cpuDecode(guest, data, cpus, ncpus) < 0) goto cleanup; virBufferVSprintf(&buf, "%s", guest->model); -- 1.6.6.1

2 2

[libvirt] [PATCH] Fix <cpu> element in domain XML schema
by Jiri Denemark 11 Feb '10

11 Feb '10

The current schema is more permissive than the XML parsing code in libvirt. Precisely, 'match' attribute is optional in schema while in reality its use is bound to <model> element: - <cpu> element without 'match' attribute is allowed only if <topology> element is the only child element of <cpu> - <cpu> element with 'match' attribute requires <model> element to be present; <topology> and <feature> elements are optional Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- docs/schemas/domain.rng | 109 +++++++++++++++++++++++++++-------------------- 1 files changed, 63 insertions(+), 46 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index bb6d00d..c913a50 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1313,52 +1313,69 @@ --> <define name="cpu"> <element name="cpu"> - <optional> - <attribute name="match"> - <choice> - <value>minimum</value> - <value>exact</value> - <value>strict</value> - </choice> - </attribute> - </optional> - <interleave> - <optional> - <element name="model"> - <text/> - </element> - </optional> - <optional> - <element name="topology"> - <attribute name="sockets"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="cores"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="threads"> - <ref name="positiveInteger"/> - </attribute> - </element> - </optional> - <zeroOrMore> - <element name="feature"> - <attribute name="policy"> - <choice> - <value>force</value> - <value>require</value> - <value>optional</value> - <value>disable</value> - <value>forbid</value> - </choice> - </attribute> - <attribute name="name"> - <ref name="featureName"/> - </attribute> - <empty/> - </element> - </zeroOrMore> - </interleave> + <choice> + <ref name="cpuTopology"/> + <group> + <ref name="cpuMatch"/> + <interleave> + <ref name="cpuModel"/> + <optional> + <ref name="cpuTopology"/> + </optional> + <zeroOrMore> + <ref name="cpuFeature"/> + </zeroOrMore> + </interleave> + </group> + </choice> + </element> + </define> + + <define name="cpuMatch"> + <attribute name="match"> + <choice> + <value>minimum</value> + <value>exact</value> + <value>strict</value> + </choice> + </attribute> + </define> + + <define name="cpuModel"> + <element name="model"> + <text/> + </element> + </define> + + <define name="cpuFeature"> + <element name="feature"> + <attribute name="policy"> + <choice> + <value>force</value> + <value>require</value> + <value>optional</value> + <value>disable</value> + <value>forbid</value> + </choice> + </attribute> + <attribute name="name"> + <ref name="featureName"/> + </attribute> + <empty/> + </element> + </define> + + <define name="cpuTopology"> + <element name="topology"> + <attribute name="sockets"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="cores"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="threads"> + <ref name="positiveInteger"/> + </attribute> </element> </define> -- 1.6.6.1

2 2

[libvirt] [PATCH 0/9] New public API for computing baseline CPU
by Jiri Denemark 11 Feb '10

11 Feb '10

Hi, This series introduces a new virConnectBaselineCPU() API call for computing baseline CPU model from a set of host CPU definitions. The call accepts a list of <cpu>...</cpu> xml definitions taken from host capabilities and returns an xml description of a guest CPU which is supported by all hosts. virsh is little bit less picky and accepts a file containing host CPU definitions. It doesn't care what is outside of the <cpu/> tag, which allows for concatenating outputs of virsh capabilities from several hosts and feeding the result to virsh cpu-baseline. This series can also be found on cpu-baseline branch of git://gitorious.org/~jirka/libvirt/jirka-staging.git In practice it works like this: # virsh cpu-baseline /dev/stdin >guest-cpu <<EOF <capabilities> <host> <cpu> <arch>x86_64</arch> <model>core2duo</model> <topology sockets='1' cores='2' threads='1'/> <feature name='lahf_lm'/> <feature name='xtpr'/> <feature name='cx16'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> </cpu> </host> </capabilities> <cpu> <arch>i686</arch> <model>pentiumpro</model> <topology sockets='1' cores='2' threads='1'/> <feature name='vme'/> <feature name='pse36'/> <feature name='pni'/> <feature name='monitor'/> <feature name='ssse3'/> </cpu> <cpu> <arch>x86_64</arch> <model>phenom</model> </cpu> EOF # cat guest-cpu <cpu match='exact'> <model>qemu32</model> <feature policy='require' name='monitor'/> <feature policy='require' name='pse36'/> </cpu> Jiri Denemark (9): Functions for computing baseline CPU from a set of host CPUs Implement cpuArchBaseline in generic CPU driver Implement cpuArchBaseline in x86 CPU driver virConnectBaselineCPU public API Internal driver API for virConnectBaselineCPU virConnectBaselineCPU public API implementation Wire protocol format and dispatcher for virConnectBaselineCPU Implement cpuBaseline in remote and qemu drivers cpu-baseline command for virsh daemon/remote.c | 25 +++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 ++ include/libvirt/libvirt.h.in | 18 +++++ src/cpu/cpu.c | 125 ++++++++++++++++++++++++++++++++++- src/cpu/cpu.h | 23 ++++++- src/cpu/cpu_generic.c | 104 ++++++++++++++++++++++++++++- src/cpu/cpu_x86.c | 88 ++++++++++++++++++++++++- src/driver.h | 6 ++ src/esx/esx_driver.c | 1 + src/libvirt.c | 57 ++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 5 ++ src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 14 ++++ src/remote/remote_driver.c | 32 +++++++++ src/remote/remote_protocol.c | 24 +++++++- src/remote/remote_protocol.h | 20 ++++++ src/remote/remote_protocol.x | 18 +++++- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + tools/virsh.c | 72 ++++++++++++++++++++ tools/virsh.pod | 8 ++ 30 files changed, 658 insertions(+), 7 deletions(-)

3 15

[libvirt] [PATCH] Add cpu_generic.c to the list of translated files
by Jiri Denemark 11 Feb '10

11 Feb '10

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- po/POTFILES.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3e6440e..6655005 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -12,6 +12,7 @@ src/conf/secret_conf.c src/conf/storage_conf.c src/conf/storage_encryption_conf.c src/cpu/cpu.c +src/cpu/cpu_generic.c src/cpu/cpu_map.c src/cpu/cpu_x86.c src/datatypes.c -- 1.6.6.1

2 1

[libvirt] [PATCH] Fix crash in LXC driver open method when URI has no path
by Daniel P. Berrange 11 Feb '10

11 Feb '10

If giving a lxc:// URI instead of lxc:/// the open method would crash ona NULL pointer * src/lxc/lxc_driver.c: Cope with a NULL URI path --- src/lxc/lxc_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1a73ca6..6d568a3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -126,7 +126,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; /* If path isn't '/' then they typoed, tell them correct path */ - if (STRNEQ(conn->uri->path, "/")) { + if (conn->uri->path != NULL && + STRNEQ(conn->uri->path, "/")) { lxcError(VIR_ERR_INTERNAL_ERROR, _("Unexpected LXC URI path '%s', try lxc:///"), conn->uri->path); -- 1.6.5.2

2 1

[libvirt] [PATCH] Fix USB device path formatting mixup
by Daniel P. Berrange 11 Feb '10

11 Feb '10

* src/util/hostusb.c: The device path for a USB device wants the bus/device IDs in decimal not octal --- src/util/hostusb.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 3cce66b..bf96539 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -184,7 +184,7 @@ usbGetDevice(unsigned bus, snprintf(dev->name, sizeof(dev->name), "%.3o:%.3o", dev->bus, dev->dev); snprintf(dev->path, sizeof(dev->path), - USB_DEVFS "%03o/%03o", dev->bus, dev->dev); + USB_DEVFS "%03d/%03d", dev->bus, dev->dev); /* XXX fixme. this should be product/vendor */ snprintf(dev->id, sizeof(dev->id), "%d %d", dev->bus, dev->dev); -- 1.6.5.2

2 1

[libvirt] [PATCH] Support 'block_passwd' command for QEMU disk encryption
by Daniel P. Berrange 11 Feb '10

11 Feb '10

The old text mode monitor prompts for a password when disks are encrypted. This interactive approach doesn't work for JSON mode monitor. Thus there is a new 'block_passwd' command that can be used. * src/qemu/qemu_driver.c: Split out code for looking up a disk secret from findVolumeQcowPassphrase, into a new method getVolumeQcowPassphrase. Enhance qemuInitPasswords() to also set the disk encryption password via the monitor * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h, src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add support for the 'block_passwd' monitor command. --- src/qemu/qemu_driver.c | 117 +++++++++++++++++++++++++++++------------- src/qemu/qemu_monitor.c | 15 +++++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 33 ++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++ src/qemu/qemu_monitor_text.h | 4 ++ 7 files changed, 188 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d77d57..03d0f5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -694,51 +694,46 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } -static virStorageEncryptionPtr -findDomainDiskEncryption(virDomainObjPtr vm, - const char *path) +static virDomainDiskDefPtr +findDomainDiskByPath(virDomainObjPtr vm, + const char *path) { - bool seen_volume; int i; - seen_volume = false; for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk; disk = vm->def->disks[i]; - if (disk->src != NULL && STREQ(disk->src, path)) { - seen_volume = true; - if (disk->encryption != NULL) - return disk->encryption; - } + if (disk->src != NULL && STREQ(disk->src, path)) + return disk; } - if (seen_volume) - qemuReportError(VIR_ERR_INVALID_DOMAIN, - _("missing <encryption> for volume %s"), path); - else - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected passphrase request for volume %s"), - path); + + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("no disk found with path %s"), + path); return NULL; } - static int -findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, - virConnectPtr conn, - virDomainObjPtr vm, - const char *path, - char **secretRet, - size_t *secretLen) +getVolumeQcowPassphrase(virConnectPtr conn, + virDomainDiskDefPtr disk, + char **secretRet, + size_t *secretLen) { - virStorageEncryptionPtr enc; virSecretPtr secret; char *passphrase; unsigned char *data; size_t size; int ret = -1; + virStorageEncryptionPtr enc; - virDomainObjLock(vm); + if (!disk->encryption) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("disk %s does not have any encryption information"), + disk->src); + return -1; + } + enc = disk->encryption; if (!conn) { qemuReportError(VIR_ERR_NO_SUPPORT, @@ -754,16 +749,12 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto cleanup; } - enc = findDomainDiskEncryption(vm, path); - if (enc == NULL) - return -1; - if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || enc->nsecrets != 1 || enc->secrets[0]->type != VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) { qemuReportError(VIR_ERR_INVALID_DOMAIN, - _("invalid <encryption> for volume %s"), path); + _("invalid <encryption> for volume %s"), disk->src); goto cleanup; } @@ -782,7 +773,7 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_FREE(data); qemuReportError(VIR_ERR_INVALID_SECRET, _("format='qcow' passphrase for %s must not contain a " - "'\\0'"), path); + "'\\0'"), disk->src); goto cleanup; } @@ -804,8 +795,30 @@ findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, ret = 0; cleanup: - virDomainObjUnlock(vm); + return ret; +} +static int +findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virConnectPtr conn, + virDomainObjPtr vm, + const char *path, + char **secretRet, + size_t *secretLen) +{ + virDomainDiskDefPtr disk; + int ret = -1; + + virDomainObjLock(vm); + disk = findDomainDiskByPath(vm, path); + + if (!disk) + goto cleanup; + + ret = getVolumeQcowPassphrase(conn, disk, secretRet, secretLen); + +cleanup: + virDomainObjUnlock(vm); return ret; } @@ -1681,8 +1694,10 @@ qemudInitCpuAffinity(virDomainObjPtr vm) static int -qemuInitPasswords(struct qemud_driver *driver, - virDomainObjPtr vm) { +qemuInitPasswords(virConnectPtr conn, + struct qemud_driver *driver, + virDomainObjPtr vm, + unsigned long long qemuCmdFlags) { int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -1698,6 +1713,36 @@ qemuInitPasswords(struct qemud_driver *driver, qemuDomainObjExitMonitorWithDriver(driver, vm); } + if (ret < 0) + goto cleanup; + + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + int i; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + char *secret; + size_t secretLen; + + if (!vm->def->disks[i]->encryption || + !vm->def->disks[i]->src) + continue; + + if (getVolumeQcowPassphrase(conn, + vm->def->disks[i], + &secret, &secretLen) < 0) + goto cleanup; + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorSetDrivePassphrase(priv->mon, + vm->def->disks[i]->info.alias, + secret); + qemuDomainObjExitMonitorWithDriver(driver, vm); + if (ret < 0) + goto cleanup; + } + } + +cleanup: return ret; } @@ -2721,7 +2766,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (qemudInitCpuAffinity(vm) < 0) goto abort; - if (qemuInitPasswords(driver, vm) < 0) + if (qemuInitPasswords(conn, driver, vm, qemuCmdFlags) < 0) goto abort; /* If we have -device, then addresses are assigned explicitly. diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 64c6cba..c1d369b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1331,3 +1331,18 @@ int qemuMonitorAddDrive(qemuMonitorPtr mon, ret = qemuMonitorTextAddDrive(mon, drivestr); return ret; } + + +int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase) +{ + DEBUG("mon=%p, fd=%d alias=%s passphrase=%p(value hidden)", mon, mon->fd, alias, passphrase); + int ret; + + if (mon->json) + ret = qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase); + else + ret = qemuMonitorTextSetDrivePassphrase(mon, alias, passphrase); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a330eff..786ad7a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -291,4 +291,8 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 032afef..c9b8d60 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1841,3 +1841,36 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + char *drive; + + if (virAsprintf(&drive, "%s%s", QEMU_DRIVE_HOST_PREFIX, alias) < 0) { + virReportOOMError(); + return -1; + } + + cmd = qemuMonitorJSONMakeCommand("block_passwd", + "s:device", drive, + "s:password", passphrase, + NULL); + VIR_FREE(drive); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ac6458c..65a70e3 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -162,4 +162,8 @@ int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index a6a4598..e993699 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2127,3 +2127,50 @@ cleanup: VIR_FREE(safe_str); return ret; } + + +int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase) +{ + char *cmd = NULL; + char *reply = NULL; + int ret = -1; + char *safe_str; + + safe_str = qemuMonitorEscapeArg(passphrase); + if (!safe_str) { + virReportOOMError(); + return -1; + } + + ret = virAsprintf(&cmd, "block_passwd %s%s \"%s\"", QEMU_DRIVE_HOST_PREFIX, alias, safe_str); + if (ret == -1) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to close fd in qemu with '%s'"), cmd); + goto cleanup; + } + + if (strstr(reply, "\nunknown command:")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("setting disk password is not supported")); + goto cleanup; + } else if (strstr(reply, "The entered password is invalid")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("the disk password is incorrect")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safe_str); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 12d75f5..1937e99 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -166,4 +166,8 @@ int qemuMonitorTextAddDevice(qemuMonitorPtr mon, int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); +int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon, + const char *alias, + const char *passphrase); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.6.5.2

2 2

[libvirt] [PATCH 5/5] macvtap support for libvirt -- schema extensions
by Stefan Berger 11 Feb '10

11 Feb '10

This part extends the domain xml schema to support the new interface type 'direct'. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH 4/5] macvtap support for libvirt -- helper code
by Stefan Berger 11 Feb '10

11 Feb '10

This part adds the helper code to setup and tear down macvtap devices using direct communication with the device driver via netlink sockets. The rather short messages received from the netlink layer are now written into a dynamically allocated buffer. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH 3/5] macvtap support for libvirt -- qemu support
by Stefan Berger 11 Feb '10

11 Feb '10

This part adds support for qemu making a macvtap tap device available via file descriptor passed to qemu command line. This also attempts to tear down the macvtap device when a VM terminates. This includes support for attachment and detachment to/from running VM. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH 2/5] macvtap support for libvirt -- parse new interface XML
by Stefan Berger 11 Feb '10

11 Feb '10

This part adds support to domain_conf.{c|h} for parsing the new interface XML of type 'direct'. The parsed mode is now stored as an int. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH 1/5] macvtap support for libvirt -- build support
by Stefan Berger 11 Feb '10

11 Feb '10

This patch adds build support for libvirt checking for certain contents of /usr/include/linux/if_link.h to see whether macvtap support is compilable on that system. One can disable macvtap support in libvirt via --without-macvtap passed to configure. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH] virStorageBackendIsMultipath: avoid dead store
by Jim Meyering 11 Feb '10

11 Feb '10

This suppresses a warning reported by clang. Since "target_type" is initialized, there's no point in storing dm_get_next_target's return value. >From d0cd9b4233194b27fb168e835223dd51e77fa6f2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Feb 2010 15:26:37 +0100 Subject: [PATCH] virStorageBackendIsMultipath: avoid dead store * src/storage/storage_backend_mpath.c (virStorageBackendIsMultipath): The result of dm_get_next_target was never used (and isn't needed), so don't store it. --- src/storage/storage_backend_mpath.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index dbcd6d8..1e7504b 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -1,7 +1,7 @@ /* * storage_backend_mpath.c: storage backend for multipath handling * - * Copyright (C) 2009-2009 Red Hat, Inc. + * Copyright (C) 2009-2010 Red Hat, Inc. * Copyright (C) 2009-2008 Dave Allan * * This library is free software; you can redistribute it and/or @@ -150,7 +150,6 @@ virStorageBackendIsMultipath(const char *devname) { int ret = 0; struct dm_task *dmt = NULL; - void *next = NULL; uint64_t start, length; char *target_type = NULL; char *params = NULL; @@ -173,8 +172,7 @@ virStorageBackendIsMultipath(const char *devname) goto out; } - next = dm_get_next_target(dmt, next, &start, &length, - &target_type, &params); + dm_get_next_target(dmt, next, &start, &length, &target_type, &params); if (target_type == NULL) { ret = -1; -- 1.7.0.rc2.170.gbc565

2 1

[libvirt] [PATCH] Add virConnectGetVersion Python API
by Taizo ITO 11 Feb '10

11 Feb '10

Hi all, The attached patch adds a new python API call for retrieving the running hypervisor version used by a connection: virConnectGetVersion. Regards, Taizo -- Taizo ITO

2 1