January 2010 - Devel - libvirt List Archives

[libvirt] [PATCH] Fix uses of virFileMakePath.
by Laine Stump 21 Jan '10

21 Jan '10

When I modified the use of virFileMakePath in my own code, I noticed that it was checking for a return < 0 to indicate error, but I had seen in virFileMakePath that it returns 0 for success, or the value of errno on failure, and errno is usually (always?) >=0. Seeing incorrect usage in one place, I investigated and found several others. --- src/lxc/lxc_container.c | 10 +++++----- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/network/bridge_driver.c | 4 ++-- src/qemu/qemu_driver.c | 8 ++++---- src/uml/uml_driver.c | 4 ++-- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 023d553..539a1f4 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -317,7 +317,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) goto err; } - if ((rc = virFileMakePath(oldroot)) < 0) { + if ((rc = virFileMakePath(oldroot)) != 0) { virReportSystemError(NULL, rc, _("Failed to create %s"), oldroot); @@ -339,7 +339,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) goto err; } - if ((rc = virFileMakePath(newroot)) < 0) { + if ((rc = virFileMakePath(newroot)) != 0) { virReportSystemError(NULL, rc, _("Failed to create %s"), newroot); @@ -407,7 +407,7 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) } for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { - if (virFileMakePath(mnts[i].dst) < 0) { + if (virFileMakePath(mnts[i].dst) != 0) { virReportSystemError(NULL, errno, _("Failed to mkdir %s"), mnts[i].src); @@ -421,7 +421,7 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) } } - if ((rc = virFileMakePath("/dev/pts") < 0)) { + if ((rc = virFileMakePath("/dev/pts") != 0)) { virReportSystemError(NULL, rc, "%s", _("Cannot create /dev/pts")); goto cleanup; @@ -510,7 +510,7 @@ static int lxcContainerMountNewFS(virDomainDefPtr vmDef) return -1; } - if (virFileMakePath(vmDef->fss[i]->dst) < 0) { + if (virFileMakePath(vmDef->fss[i]->dst) != 0) { virReportSystemError(NULL, errno, _("Failed to create %s"), vmDef->fss[i]->dst); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3cecdce..6304815 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -556,7 +556,7 @@ lxcControllerRun(virDomainDefPtr def, goto cleanup; } - if (virFileMakePath(devpts) < 0) { + if (virFileMakePath(devpts) != 0) { virReportSystemError(NULL, errno, _("Failed to make path %s"), devpts); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index ec5ee6b..d78f7f7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1193,7 +1193,7 @@ static int lxcVmStart(virConnectPtr conn, char **veths = NULL; lxcDomainObjPrivatePtr priv = vm->privateData; - if ((r = virFileMakePath(driver->logDir)) < 0) { + if ((r = virFileMakePath(driver->logDir)) != 0) { virReportSystemError(conn, r, _("Cannot create log directory '%s'"), driver->logDir); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4385215..08e5ee6 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -539,13 +539,13 @@ dhcpStartDhcpDaemon(virConnectPtr conn, return -1; } - if ((err = virFileMakePath(NETWORK_PID_DIR)) < 0) { + if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(conn, err, _("cannot create directory %s"), NETWORK_PID_DIR); return -1; } - if ((err = virFileMakePath(NETWORK_STATE_DIR)) < 0) { + if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(conn, err, _("cannot create directory %s"), NETWORK_STATE_DIR); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 951e033..fa2e5d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1033,19 +1033,19 @@ qemudStartup(int privileged) { goto out_of_memory; } - if (virFileMakePath(qemu_driver->stateDir) < 0) { + if (virFileMakePath(qemu_driver->stateDir) != 0) { char ebuf[1024]; VIR_ERROR(_("Failed to create state dir '%s': %s"), qemu_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } - if (virFileMakePath(qemu_driver->libDir) < 0) { + if (virFileMakePath(qemu_driver->libDir) != 0) { char ebuf[1024]; VIR_ERROR(_("Failed to create lib dir '%s': %s"), qemu_driver->libDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } - if (virFileMakePath(qemu_driver->cacheDir) < 0) { + if (virFileMakePath(qemu_driver->cacheDir) != 0) { char ebuf[1024]; VIR_ERROR(_("Failed to create cache dir '%s': %s"), qemu_driver->cacheDir, virStrerror(errno, ebuf, sizeof ebuf)); @@ -2782,7 +2782,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, vm->def->graphics[0]->data.vnc.port = port; } - if (virFileMakePath(driver->logDir) < 0) { + if (virFileMakePath(driver->logDir) != 0) { virReportSystemError(conn, errno, _("cannot create log directory %s"), driver->logDir); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index b808090..31cea5c 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -419,7 +419,7 @@ umlStartup(int privileged) { goto error; } - if (virFileMakePath(uml_driver->monitorDir) < 0) { + if (virFileMakePath(uml_driver->monitorDir) != 0) { char ebuf[1024]; VIR_ERROR(_("Failed to create monitor directory %s: %s"), uml_driver->monitorDir, virStrerror(errno, ebuf, sizeof ebuf)); @@ -840,7 +840,7 @@ static int umlStartVMDaemon(virConnectPtr conn, return -1; } - if (virFileMakePath(driver->logDir) < 0) { + if (virFileMakePath(driver->logDir) != 0) { virReportSystemError(conn, errno, _("cannot create log directory %s"), driver->logDir); -- 1.6.6

2 2

[libvirt] [PATCH 0/4] Take 3 - Patchset to avoid uid problems in volume creation
by Laine Stump 21 Jan '10

21 Jan '10

The last iteration of this work (which is now completely obsoleted) is at https://www.redhat.com/archives/libvir-list/2010-January/msg00302.html These patches are intended in particular to deal with problems creating storage volumes on pools that are located on root-squashing NFS servers, but hopefully they are overall just "a good thing" and will solve other as-yet-unencountered/unreported problems. Differences from previous version of the patches: 1) For completeness, chmod is done inside virFileCreate and virDirCreate as well as setting uid and gid. 2) Both of those functions now take an additional flag - VIR_FILE_CREATE_ALLOW_EXIST, which does exactly what you'd think. 3) I now always attempt to set uid/gid correctly, even if not running as root (per Serge Hallyn's suggestions during the last go around) 4) I've added a patch that uses similar logic to get the uid/gid/mode of the directories for newly created pools correct as well. (Previously, we'd been ignoring the permission bits of pool definition xml).

2 13

[libvirt] [PATCH 1/3] Make sure that the vmx2xml test data files are shipped in the distribution.
by Diego Elio Pettenò 20 Jan '10

20 Jan '10

Without these files the tests will fail when ESX support is enabled. --- tests/Makefile.am | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 63 insertions(+), 1 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 335b0e5..cf51162 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -47,6 +47,67 @@ qemuhelpdata = \ qemu-kvm-0.10.5 \ qemu-kvm-0.11.0-rc2 +vmx2xmldata = \ + vmx2xml-cdrom-ide-device.vmx \ + vmx2xml-cdrom-ide-device.xml \ + vmx2xml-cdrom-ide-file.vmx \ + vmx2xml-cdrom-ide-file.xml \ + vmx2xml-cdrom-scsi-device.vmx \ + vmx2xml-cdrom-scsi-device.xml \ + vmx2xml-cdrom-scsi-file.vmx \ + vmx2xml-cdrom-scsi-file.xml \ + vmx2xml-esx-in-the-wild-1.vmx \ + vmx2xml-esx-in-the-wild-1.xml \ + vmx2xml-esx-in-the-wild-2.vmx \ + vmx2xml-esx-in-the-wild-2.xml \ + vmx2xml-esx-in-the-wild-3.vmx \ + vmx2xml-esx-in-the-wild-3.xml \ + vmx2xml-esx-in-the-wild-4.vmx \ + vmx2xml-esx-in-the-wild-4.xml \ + vmx2xml-ethernet-bridged.vmx \ + vmx2xml-ethernet-bridged.xml \ + vmx2xml-ethernet-custom.vmx \ + vmx2xml-ethernet-custom.xml \ + vmx2xml-ethernet-e1000.vmx \ + vmx2xml-ethernet-e1000.xml \ + vmx2xml-floppy-device.vmx \ + vmx2xml-floppy-device.xml \ + vmx2xml-floppy-file.vmx \ + vmx2xml-floppy-file.xml \ + vmx2xml-gsx-in-the-wild-1.vmx \ + vmx2xml-gsx-in-the-wild-1.xml \ + vmx2xml-gsx-in-the-wild-2.vmx \ + vmx2xml-gsx-in-the-wild-2.xml \ + vmx2xml-gsx-in-the-wild-3.vmx \ + vmx2xml-gsx-in-the-wild-3.xml \ + vmx2xml-gsx-in-the-wild-4.vmx \ + vmx2xml-gsx-in-the-wild-4.xml \ + vmx2xml-harddisk-ide-file.vmx \ + vmx2xml-harddisk-ide-file.xml \ + vmx2xml-harddisk-scsi-file.vmx \ + vmx2xml-harddisk-scsi-file.xml \ + vmx2xml-minimal-64bit.vmx \ + vmx2xml-minimal-64bit.xml \ + vmx2xml-minimal.vmx \ + vmx2xml-minimal.xml \ + vmx2xml-parallel-device.vmx \ + vmx2xml-parallel-device.xml \ + vmx2xml-parallel-file.vmx \ + vmx2xml-parallel-file.xml \ + vmx2xml-scsi-buslogic.vmx \ + vmx2xml-scsi-buslogic.xml \ + vmx2xml-scsi-writethrough.vmx \ + vmx2xml-scsi-writethrough.xml \ + vmx2xml-serial-device.vmx \ + vmx2xml-serial-device.xml \ + vmx2xml-serial-file.vmx \ + vmx2xml-serial-file.xml \ + vmx2xml-serial-pipe-client-app.vmx \ + vmx2xml-serial-pipe-client-vm.vmx \ + vmx2xml-serial-pipe-server-app.vmx \ + vmx2xml-serial-pipe-server-vm.vmx \ + vmx2xml-serial-pipe.xml + EXTRA_DIST = \ oomtrace.pl \ test-lib.sh \ @@ -69,7 +130,8 @@ EXTRA_DIST = \ storagevolxml2xmlin \ nodedevschematest \ nodedevschemadata \ - $(patsubst %,qemuhelpdata/%,$(qemuhelpdata)) + $(patsubst %,qemuhelpdata/%,$(qemuhelpdata)) \ + $(patsubst %,vmx2xmldata/%,$(vmx2xmldata)) noinst_PROGRAMS = virshtest conftest \ nodeinfotest statstest qparamtest -- 1.6.6.rc4

3 8

[libvirt] [PATCH] qemu: Don't allocate zero bytes
by Matthias Bolte 20 Jan '10

20 Jan '10

--- src/qemu/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 38eb3fd..92a9348 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -836,7 +836,7 @@ qemudCapsInitGuest(virCapsPtr caps, return -1; } - if (VIR_ALLOC_N(machines, nmachines) < 0) { + if (VIR_ALLOC_N(machines, 1) < 0) { virReportOOMError(NULL); VIR_FREE(machine->name); VIR_FREE(machine); -- 1.6.3.3

2 2

[libvirt] [PATCH] node_device_linux_sysfs.c: avoid opendir/fd leak on error path
by Jim Meyering 20 Jan '10

20 Jan '10

This looks pretty clear. It calls closedir upon successful return, but not in the error case. >From 526a2bc2d87f0bf7a0c16a2a96316e5c1d5dae2e Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 20 Jan 2010 17:49:35 +0100 Subject: [PATCH] node_device_linux_sysfs.c: avoid opendir/fd leak on error path * src/node_device/node_device_linux_sysfs.c(get_virtual_functions_linux): Don't leak a DIR buffer and file descriptor on error path. --- src/node_device/node_device_linux_sysfs.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index ff7aaf0..e611e1a 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -2,7 +2,7 @@ * node_device_hal_linuc.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009 Red Hat, Inc. + * Copyright (C) 2009-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 @@ -371,6 +371,8 @@ int get_virtual_functions_linux(const char *sysfs_path, ret = 0; out: + if (dir) + closedir(dir); VIR_FREE(device_link); return 0; } -- 1.6.6.516.gb72f

2 4

[libvirt] [PATCH] domain_conf.c: avoid a leak and the need for "cleanup:" block
by Jim Meyering 20 Jan '10

20 Jan '10

This avoids a leak and the need for a "cleanup:" block, along with its three goto statements. While often I prefer to write functions with a single return point, this one no longer has the need, now that "addr" is freed immediately after allocation. Adding the semicolon in the "case..." stmt may look odd. It's there because the first stmt is the declaration of "port". If you'd prefer, an alternative is to put the contents of that case inside a {...} block. >From dc879b00ccf6cc451b95ca3712e647605579ee0b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 20 Jan 2010 19:27:43 +0100 Subject: [PATCH] domain_conf.c: avoid a leak and the need for "cleanup:" block * src/conf/domain_conf.c (virDomainChrDefFormat): Plug a leak on an error path, and at the same time, eliminate the need for a "cleanup:" block. Before, the "return -1" after the switch would leak an "addr" string. Now, by reversing the port,addr- getting blocks we can free "addr" immediately and skip the goto. --- src/conf/domain_conf.c | 30 +++++++++++++----------------- 1 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6254dc8..7815805 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1,7 +1,7 @@ /* * domain_conf.c: domain XML processing * - * Copyright (C) 2006-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 @@ -4831,7 +4831,6 @@ virDomainChrDefFormat(virConnectPtr conn, const char *targetName = virDomainChrTargetTypeToString(def->targetType); const char *elementName; - const char *addr = NULL; int ret = 0; switch (def->targetType) { @@ -4847,8 +4846,7 @@ virDomainChrDefFormat(virConnectPtr conn, if (!type) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, _("unexpected char type %d"), def->type); - ret = -1; - goto cleanup; + return -1; } /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ @@ -4931,22 +4929,23 @@ virDomainChrDefFormat(virConnectPtr conn, switch (def->targetType) { case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD: - addr = virSocketFormatAddr(def->target.addr); - if (addr == NULL) { - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to format guestfwd address")); - ret = -1; - goto cleanup; - } + ; /* dummy stmt, for following declaration */ int port = virSocketGetPort(def->target.addr); if (port < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to format guestfwd port")); - ret = -1; - goto cleanup; + return -1; + } + const char *addr = virSocketFormatAddr(def->target.addr); + if (addr == NULL) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to format guestfwd address")); + return -1; } - virBufferVSprintf(buf, " <target type='guestfwd' address='%s' port='%d'/>\n", + virBufferVSprintf(buf, + " <target type='guestfwd' address='%s' port='%d'/>\n", addr, port); + VIR_FREE(addr); break; case VIR_DOMAIN_CHR_TARGET_TYPE_PARALLEL: @@ -4969,9 +4968,6 @@ virDomainChrDefFormat(virConnectPtr conn, virBufferVSprintf(buf, " </%s>\n", elementName); -cleanup: - VIR_FREE(addr); - return ret; } -- 1.6.6.516.gb72f

2 3

Re: [libvirt] [PATCH] Document cpu-compare command in virsh man page
by Jiri Denemark 20 Jan '10

20 Jan '10

On Wed, Jan 20, 2010 at 16:17:32 +0100, Daniel Veillard wrote: > On Wed, Jan 20, 2010 at 03:16:29PM +0100, Jiri Denemark wrote: > > Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> > > --- > > tools/virsh.pod | 9 +++++++++ > > 1 files changed, 9 insertions(+), 0 deletions(-) > > > > diff --git a/tools/virsh.pod b/tools/virsh.pod > > index 84a8564..10f622f 100644 > > --- a/tools/virsh.pod > > +++ b/tools/virsh.pod > > @@ -213,6 +213,15 @@ crashed. > > Prints the available amount of memory on the machine or within a > > NUMA cell if I<cellno> is provided. > > > > +=item B<cpu-compare> I<FILE> > > + > > +Compare CPU definition from XML <file> with host CPU. The XML <file> may > > +contain either host or guest CPU definition. The host CPU definition is the > > +<cpu> element and its contents as printed by B<capabilities> command. The > > +guest CPU definition is the <cpu> element and its contents from domain XML > > +definition. For more information on guest CPU definition see: > > +L<http://libvirt.org/formatdomain.html#elementsCPU> > > + > > =back > > > > =head1 DOMAIN COMMANDS > > ACK, Thanks, pushed. Jirka

1 0

[libvirt] [PATCH] Document cpu-compare command in virsh man page
by Jiri Denemark 20 Jan '10

20 Jan '10

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- tools/virsh.pod | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 84a8564..10f622f 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -213,6 +213,15 @@ crashed. Prints the available amount of memory on the machine or within a NUMA cell if I<cellno> is provided. +=item B<cpu-compare> I<FILE> + +Compare CPU definition from XML <file> with host CPU. The XML <file> may +contain either host or guest CPU definition. The host CPU definition is the +<cpu> element and its contents as printed by B<capabilities> command. The +guest CPU definition is the <cpu> element and its contents from domain XML +definition. For more information on guest CPU definition see: +L<http://libvirt.org/formatdomain.html#elementsCPU> + =back =head1 DOMAIN COMMANDS -- 1.6.6

1 0

[libvirt] [PATCH] Libvirt Snapshot API
by Philip Jameson 20 Jan '10

20 Jan '10

A little while ago I submitted a patch to add a snapshot API, but I was informed that it got somewhat lost in the list. I have attached a patch that implements this API at least for the qemu driver, and it also added another function for screenshots, just because it usually is nice to get a view of the screen with the snapshot. The snapshot functions just take names, as I figure it should be up to libvirt to figure out where the snapshots go. e.g. for qemu, they just get stored with the drive using qemu's built in facilities. If someone were using LVM, then that could be managed by libvirt, just like it does for qemu monitors and such. The 'list snapshots' just returns a list of names for now, as that's all I needed, but I think it would probably be best to add a struct that would be name/time. I don't know if it's of any use at the moment, but I needed these functions implemented, so I figured I'd at least submit my changes Philip Jameson

2 3

[libvirt] XML for the "boot=on" option
by Shi Jin 20 Jan '10

20 Jan '10

Hi, there, In order to install windows 2008 using the virtio driver, I had to modify the KVM command to include the boot=on option like: -drive file=<path>/virtioWin08.raw,if=virtio,index=0,boot=on However, I haven't been able to find a corresponding libvirt XML element that can enable this "boot=on" option. Is there a way to do this? Thanks a lot. -- Shi Jin, PhD

2 1