August 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] command: shell-quote when logging commands
by Eric Blake 31 Aug '12

31 Aug '12

Without this patch, logged command executions can be ambiguous if the command contained any shell metacharacters. This has caused more than one person to attempt to patch clients to add unnecessary quoting, without realizing that the command itself was run with correct args, and only the logged output was ambiguous. * src/util/command.c (virCommandToString): Add shell escapes. * tests/commandtest.c (test16): Test new behavior. * tests/commanddata/test16.log: Update expected output. * tests/qemuxml2argvdata/qemuxml2argv-*.args: Likewise. * tests/networkxml2argvdata/*.argv: Likewise. --- src/util/command.c | 25 ++++++++++++++++------ tests/commanddata/test16.log | 2 +- tests/commandtest.c | 6 ++++-- .../nat-network-dns-txt-record.argv | 2 +- .../qemuxml2argv-disk-drive-network-rbd-auth.args | 7 +++--- .../qemuxml2argv-disk-drive-network-rbd.args | 7 +++--- .../qemuxml2argv-graphics-vnc.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 6 +++--- 9 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 49ec178..418b198 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1614,9 +1614,10 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd) * virCommandToString: * @cmd: the command to convert * - * Call after adding all arguments and environment settings, but before - * Run/RunAsync, to return a string representation of the environment and - * arguments of cmd. If virCommandRun cannot succeed (because of an + * Call after adding all arguments and environment settings, but + * before Run/RunAsync, to return a string representation of the + * environment and arguments of cmd, suitably quoted for pasting into + * a shell. If virCommandRun cannot succeed (because of an * out-of-memory condition while building cmd), NULL will be returned. * Caller is responsible for freeing the resulting string. */ @@ -1639,13 +1640,25 @@ virCommandToString(virCommandPtr cmd) } for (i = 0; i < cmd->nenv; i++) { - virBufferAdd(&buf, cmd->env[i], strlen(cmd->env[i])); + /* In shell, a='b c' has a different meaning than 'a=b c', so + * we must determine where the '=' lives. */ + char *eq = strchr(cmd->env[i], '='); + + if (!eq) { + virBufferFreeAndReset(&buf); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid use of command API")); + return NULL; + } + eq++; + virBufferAdd(&buf, cmd->env[i], eq - cmd->env[i]); + virBufferEscapeShell(&buf, eq); virBufferAddChar(&buf, ' '); } - virBufferAdd(&buf, cmd->args[0], strlen(cmd->args[0])); + virBufferEscapeShell(&buf, cmd->args[0]); for (i = 1; i < cmd->nargs; i++) { virBufferAddChar(&buf, ' '); - virBufferAdd(&buf, cmd->args[i], strlen(cmd->args[i])); + virBufferEscapeShell(&buf, cmd->args[i]); } if (virBufferError(&buf)) { diff --git a/tests/commanddata/test16.log b/tests/commanddata/test16.log index 7088165..119dd29 100644 --- a/tests/commanddata/test16.log +++ b/tests/commanddata/test16.log @@ -1 +1 @@ -A=B true C +A=B C=D E true F G H diff --git a/tests/commandtest.c b/tests/commandtest.c index b1c7523..c005153 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -607,12 +607,14 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) { virCommandPtr cmd = virCommandNew("true"); char *outactual = NULL; - const char *outexpect = "A=B true C"; + const char *outexpect = "A=B C='D E' true F 'G H'"; int ret = -1; int fd = -1; virCommandAddEnvPair(cmd, "A", "B"); - virCommandAddArg(cmd, "C"); + virCommandAddEnvPair(cmd, "C", "D E"); + virCommandAddArg(cmd, "F"); + virCommandAddArg(cmd, "G H"); if ((outactual = virCommandToString(cmd)) == NULL) { virErrorPtr err = virGetLastError(); diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv index 1b31871..2a6c799 100644 --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -1,6 +1,6 @@ @DNSMASQ@ --strict-order --bind-interfaces \ --local=// --domain-needed --filterwin2k --conf-file= \ ---except-interface lo --txt-record=example,example value \ +--except-interface lo '--txt-record=example,example value' \ --listen-address 192.168.122.1 --listen-address 192.168.123.1 \ --listen-address 2001:db8:ac10:fe01::1 \ --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args index b323e91..02a9869 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args @@ -2,9 +2,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive \ file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=rbd:pool/image:\ +'file=rbd:pool/image:\ id=myname:\ key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ auth_supported=cephx\;none:\ -mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\ -if=virtio,format=raw -net none -serial none -parallel none -usb +mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\ +mon3.example.org\:6322,\ +if=virtio,format=raw' -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args index 69cf7c7..61c8f7d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args @@ -2,6 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive \ file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=rbd:pool/image:auth_supported=none:\ -mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\ -if=virtio,format=raw -net none -serial none -parallel none -usb +'file=rbd:pool/image:auth_supported=none:\ +mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\ +mon3.example.org\:6322,\ +if=virtio,format=raw' -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args index 2af1540..af99225 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -monitor unix:/tmp/test-monitor,server,\ nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none \ --parallel none -usb -vnc [2001:1:2:3:4:5:1234:1234]:3 +-parallel none -usb -vnc '[2001:1:2:3:4:5:1234:1234]:3' diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args index 19450a1..88bdd13 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args @@ -1,4 +1,4 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test NS=ns BAR= \ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test NS=ns BAR='' \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -unknown \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args index 3f6cb81..ac28bad 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ -pc -m 214 -smp 1 -smbios type=0,vendor=LENOVO,version=6FET82WW (3.12 ) -smbios \ -type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ +pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \ +-smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\ serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\ -uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat \ +uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -- 1.7.11.4

2 2

[libvirt] [PATCH] qemu: fix remote port searching
by Martin Kletzander 31 Aug '12

31 Aug '12

After fixing the last review comments on remote port searching (commit a14b4aea512d6c3a42af56207a65ef10ac4a12a1), the commit right after that wasn't modified accordingly, therefore two values weren't changed as they should and the configurable ports don't work as expected. This simple commit changes last two values missed and fixes the issue. --- This time I double checked that it does what it should! src/qemu/qemu_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7f85aea..5c3b89e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3467,7 +3467,7 @@ int qemuProcessStart(virConnectPtr conn, if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && !vm->def->graphics[0]->data.vnc.socket && vm->def->graphics[0]->data.vnc.autoport) { - int port = qemuProcessNextFreePort(driver, QEMU_REMOTE_PORT_MIN); + int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); @@ -3478,7 +3478,7 @@ int qemuProcessStart(virConnectPtr conn, int port = -1; if (vm->def->graphics[0]->data.spice.autoport || vm->def->graphics[0]->data.spice.port == -1) { - port = qemuProcessNextFreePort(driver, QEMU_REMOTE_PORT_MIN); + port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.7.12

3 5

[libvirt] [PATCH 0/2] virsh: Check for connection usability before each command
by Peter Krempa 31 Aug '12

31 Aug '12

Some commands in virsh forgot to do this check before executing remote commands and printed unexpected error messages. This series moves the connection check to a central point so that these mistakes don't happen. Also it's a nice cleanup :). Peter Krempa (2): virsh: Improve checking for connection when running commands virsh: remove unneeded usage of vshConnectionUsability() tools/virsh-domain-monitor.c | 36 -------- tools/virsh-domain.c | 215 ------------------------------------------- tools/virsh-host.c | 40 -------- tools/virsh-interface.c | 40 -------- tools/virsh-network.c | 35 ------- tools/virsh-nodedev.c | 18 ---- tools/virsh-nwfilter.c | 15 --- tools/virsh-pool.c | 57 ------------ tools/virsh-secret.c | 18 ---- tools/virsh-snapshot.c | 30 ------ tools/virsh-volume.c | 50 ---------- tools/virsh.c | 46 +++++---- tools/virsh.h | 1 - 13 files changed, 28 insertions(+), 573 deletions(-) -- 1.7.12

2 4

[libvirt] [PATCH] schemas: Fix wwn pattern
by Osier Yang 31 Aug '12

31 Aug '12

It should be [] instead of (). --- docs/schemas/nodedev.rng | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index c07a97d..88a4e9d 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -247,7 +247,7 @@ <define name='wwn'> <data type='string'> - <param name='pattern'>(0-9a-fA-F){16}</param> + <param name='pattern'>[0-9a-fA-F]{16}</param> </data> </define> -- 1.7.7.3

2 2

[libvirt] [PATCH] util: Update the inconsistent and outdated comments
by Osier Yang 31 Aug '12

31 Aug '12

The codes were updated to allow to reset the device as long as there is no devices/functions behind the same bus. However, the comments were kept without touched. --- Pushed under trivial rule. --- src/util/pci.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 137521b..0742d07 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -605,10 +605,9 @@ pciTrySecondaryBusReset(pciDevice *dev, uint16_t ctl; int ret = -1; - /* For now, we just refuse to do a secondary bus reset - * if there are other devices/functions behind the bus. - * In future, we could allow it so long as those devices - * are not in use by the host or other guests. + /* Refuse to do a secondary bus reset if there are other + * devices/functions behind the bus are used by the host + * or other guests. */ if ((conflict = pciBusContainsActiveDevices(dev, inactiveDevs))) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.7.7.3

1 0

[libvirt] [PATCH 0/2] conf: Fix parsing and formatting of seclabels
by Jiri Denemark 31 Aug '12

31 Aug '12

This patchset contains updated patches from Marcelo with the changes I sent previously as a WIP. Marcelo Cerri (2): conf: Fix parsing of seclabels without model conf: Avoid formatting auto-generated DAC labels src/conf/domain_conf.c | 78 ++++++++++++++++++++++++++++++++++---------------- src/conf/domain_conf.h | 1 + 2 files changed, 55 insertions(+), 24 deletions(-) -- 1.7.12

2 4

[libvirt] [PATCH] virsh: fixed incorrect timing report
by Viktor Mihajlovski 31 Aug '12

31 Aug '12

When executing virsh -t <command> the reported timing was off by 3 orders of magnitude if the command took more than one second. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7a5b92c..04d1b52 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1509,7 +1509,7 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) return ret; if (enable_timing) { - double diff_ms = (((after.tv_sec - before.tv_sec) * 1000000.0) + + double diff_ms = (((after.tv_sec - before.tv_sec) * 1000.0) + ((after.tv_usec - before.tv_usec) / 1000.0)); vshPrint(ctl, _("\n(Time: %.3f ms)\n\n"), diff_ms); -- 1.7.0.4

3 2

[libvirt] [PATCH] virsh: Document subdriver option of attach-disk
by Jiri Denemark 31 Aug '12

31 Aug '12

--- tools/virsh.pod | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index cad5b1c..a26c420 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1675,9 +1675,15 @@ I<source> is path for the files and devices. I<target> controls the bus or device under which the disk is exposed to the guest OS. It indicates the "logical" device name. I<driver> can be I<file>, I<tap> or I<phy> for the Xen hypervisor depending on the kind of access; or I<qemu> for the QEMU emulator. -I<type> can indicate I<lun>, I<cdrom> or I<floppy> as alternative to the disk default, -although this use only replaces the media within the existing virtual cdrom or -floppy device; consider using B<update-device> for this usage instead. +Further details to the driver can be passed using I<subdriver>. For Xen +I<subdriver> can be I<aio>, while for QEMU subdriver should match the format +of the disk source, such as I<raw> or I<qcow2>. Hypervisor default will be +used if I<subdriver> is not specified. However, the default may not be +correct, esp. for QEMU as for security reasons it is configured not to detect +disk formats. I<type> can indicate I<lun>, I<cdrom> or I<floppy> as +alternative to the disk default, although this use only replaces the media +within the existing virtual cdrom or floppy device; consider using +B<update-device> for this usage instead. I<mode> can specify the two specific mode I<readonly> or I<shareable>. I<--config> indicates the changes will affect the next boot of the domain, for compatibility purposes, I<--persistent> is alias of I<--config>. -- 1.7.12

2 2

[libvirt] [PATCH] qemu: Don't ignore CPU tuning config if required cgroups are missing
by Jiri Denemark 31 Aug '12

31 Aug '12

--- src/qemu/qemu_cgroup.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index df67ff3..7298e28 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -546,16 +546,21 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; - if (driver->cgroup == NULL) - return 0; /* Not supported, so claim success */ - if ((period || quota) && - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_SYSTEM_ERROR, "%s", - _("cgroup cpu is not active")); + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); return -1; } + /* We are trying to setup cgroups for CPU pinning, which can also be done + * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal + * here. + */ + if (driver->cgroup == NULL) + return 0; + rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); if (rc != 0) { virReportSystemError(-rc, @@ -636,6 +641,14 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, long long quota = vm->def->cputune.emulator_quota; int rc, i; + if ((period || quota) && + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); + return -1; + } + if (driver->cgroup == NULL) return 0; /* Not supported, so claim success */ @@ -656,10 +669,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!qemuCgroupControllerActive(driver, i)) { - VIR_WARN("cgroup %d is not active", i); + if (!qemuCgroupControllerActive(driver, i)) continue; - } rc = virCgroupMoveTask(cgroup, cgroup_emulator, i); if (rc < 0) { virReportSystemError(-rc, -- 1.7.12

2 2

[libvirt] [PATCH 0/2] WIP: additional fixes to the security driver
by Jiri Denemark 31 Aug '12

31 Aug '12

Hi, these patches are a work in progress and apply on top of the two recent patches from Marcelo. Jiri Denemark (2): Fix seclabels parsing Format seclabels in a compatible way src/conf/domain_conf.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) -- 1.7.12

1 3