- Devel - libvirt List Archives

[libvirt] [PATCH]daemon: don't free domain if it's null
by Chen Hanxiao 18 Oct '13

18 Oct '13

From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> If we fail to get domain, we had to judge whether it's null or not when doing 'cleanup'. Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> --- daemon/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index f3de6a0..decaecc 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4643,7 +4643,8 @@ lxcDispatchDomainOpenNamespace(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virDomainFree(dom); + if (dom) + virDomainFree(dom); return rv; } -- 1.8.2.1

2 1

[libvirt] [PATCH] Fix typo s/SASL_CONF_DIR/SASL_CONF_PATH/ in QEMU VNC code
by Daniel P. Berrange 18 Oct '13

18 Oct '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The QEMU VNC client arg code has a long standing typo of SASL_CONF_DIR when it should be SASL_CONFIG_PATH for the env variable name. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 814f368..5e6b0c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7125,7 +7125,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",sasl"); if (cfg->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", cfg->vncSASLdir); + virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir); /* TODO: Support ACLs later */ } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args index 67ef88f..239fde1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ -SASL_CONF_DIR=/root/.sasl2 QEMU_AUDIO_DRV=none \ +SASL_CONF_PATH=/root/.sasl2 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 -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args index d71a998..c681b1b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ -SASL_CONF_DIR=/root/.sasl2 QEMU_AUDIO_DRV=none \ +SASL_CONF_PATH=/root/.sasl2 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 -usb -hda \ /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \ -- 1.8.3.1

3 2

[libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for attach-disk command
by Chen Hanxiao 18 Oct '13

18 Oct '13

From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> '--print-xml' option is very useful for doing some test. But we had to specify a real domain for it. This patch could enable us to specify a fake domain when using --print-xml option. Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> --- tools/virsh-domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2aed9f9..565966d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -528,13 +528,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (live) flags |= VIR_DOMAIN_AFFECT_LIVE; - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - return false; - - if (persistent && - virDomainIsActive(dom) == 1) - flags |= VIR_DOMAIN_AFFECT_LIVE; - if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 || vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 || vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 || @@ -672,6 +665,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) goto cleanup; } + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (persistent && + virDomainIsActive(dom) == 1) + flags |= VIR_DOMAIN_AFFECT_LIVE; + if (flags) ret = virDomainAttachDeviceFlags(dom, xml, flags); else -- 1.8.2.1

2 2

[libvirt] [PATCH 0/3] esx: Remove unnecessary NULL comparisons
by Geoff Hickey 17 Oct '13

17 Oct '13

In reply to my last submit, Eric Blake suggested removing an explicit NULL comparison, and instead to simply use the pointer in a boolean context, as in: if (ptr) instead of if (ptr != NULL). Since the second form was used thoughout the esx code, making this change in one place wouldn't have advanced the cause of consistency in the code. This series of patches makes this change throughout the esx code. There are no logic changes. The result is (arguably) easier to read. Geoff Hickey (3): esx: Remove unnecessary NULL comparisons (1/3) esx: Remove unnecessary NULL comparisons (2/3) esx: Remove unnecessary NULL comparisons (3/3) src/esx/esx_driver.c | 244 +++++++++--------- src/esx/esx_interface_driver.c | 10 +- src/esx/esx_network_driver.c | 64 ++--- src/esx/esx_storage_backend_iscsi.c | 44 ++-- src/esx/esx_storage_backend_vmfs.c | 86 +++---- src/esx/esx_storage_driver.c | 6 +- src/esx/esx_util.c | 48 ++-- src/esx/esx_vi.c | 475 ++++++++++++++++++------------------ src/esx/esx_vi_methods.c | 10 +- src/esx/esx_vi_types.c | 88 +++---- 10 files changed, 535 insertions(+), 540 deletions(-) -- 1.8.1.2

2 5

[libvirt] [PATCH] Remove ATTRIBUTE_NONNULL(3) from qemuMonitorJSONDrivePivot
by John Ferlan 17 Oct '13

17 Oct '13

The header definition didn't match the function declaration, so adjusted header to reflect the definition. Found during a Coverity build where STATIC_ANALYSIS is enabled resulting in the internal.h adding __nonnull__ handling to arguments. Commit '6d264c91' added support for the qemuMonitorJSONDrivePivot() and commit 'fbc3adc9' added a corresponding test which ended up triggering the build failure which I didn't notice until today! Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- src/qemu/qemu_monitor_json.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 51cf19c..a1a7548 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -250,7 +250,7 @@ int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, const char *device, const char *file, const char *format) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, -- 1.8.3.1

2 1

[libvirt] [PATCH] Add support for SPICE SASL
by Christophe Fergeau 17 Oct '13

17 Oct '13

This adds a spice_sasl boolean option to qemu.conf to mimic what is donc for VNC SASL support. I did not add a spice_sasl_dir option as the corresponding vnc_sasl_dir option sets a SASL_CONF_DIR environment variable, but I could not find any reference to that variable in either QEMU or cyrus-sasl, and Google was not helpful either. --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 11 ++++++ src/qemu/qemu_command.c | 2 + src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + .../qemuxml2argv-graphics-spice-sasl.args | 12 ++++++ .../qemuxml2argv-graphics-spice-sasl.xml | 45 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 9 files changed, 80 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 32db983..3dc1b43 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -38,6 +38,7 @@ module Libvirtd_qemu = | bool_entry "spice_tls" | str_entry "spice_tls_x509_cert_dir" | str_entry "spice_password" + | bool_entry "spice_sasl" let nogfx_entry = bool_entry "nographics_allow_host_audio" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index bf57b9c..7b128aa 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -140,6 +140,17 @@ #spice_password = "XYZ12345" +# Enable use of SASL encryption on the SPICE server. This requires +# a SPICE client which supports the SASL protocol extension. +# Examples include vinagre, virt-viewer and virt-manager +# itself. +# +# It is necessary to configure /etc/sasl2/qemu.conf to choose +# the desired SASL plugin (eg, GSSPI for Kerberos) +# +#spice_sasl = 1 + + # By default, if no graphical front end is configured, libvirt will disable # QEMU audio output since directly talking to alsa/pulseaudio may not work # with various security settings. If you know what you're doing, enable diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abb62e9..ea5cfcb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7340,6 +7340,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, * migration algorithm silently. */ virBufferAddLit(&opt, ",seamless-migration=on"); } + if (cfg->spiceSASL) + virBufferAddLit(&opt, ",sasl"); virCommandAddArg(cmd, "-spice"); virCommandAddArgBuffer(cmd, &opt); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 44a2296..532b0ff 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -399,6 +399,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_STR("spice_tls_x509_cert_dir", cfg->spiceTLSx509certdir); GET_VALUE_STR("spice_listen", cfg->spiceListen); GET_VALUE_STR("spice_password", cfg->spicePassword); + GET_VALUE_BOOL("spice_sasl", cfg->spiceSASL); GET_VALUE_LONG("remote_websocket_port_min", cfg->webSocketPortMin); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index ea3c691..35a2515 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -110,6 +110,7 @@ struct _virQEMUDriverConfig { char *vncSASLdir; bool spiceTLS; + bool spiceSASL; char *spiceTLSx509certdir; char *spiceListen; char *spicePassword; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 7af3f64..f759db5 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -15,6 +15,7 @@ module Test_libvirtd_qemu = { "spice_tls" = "1" } { "spice_tls_x509_cert_dir" = "/etc/pki/libvirt-spice" } { "spice_password" = "XYZ12345" } +{ "spice_sasl" = "1" } { "nographics_allow_host_audio" = "1" } { "remote_display_port_min" = "5900" } { "remote_display_port_max" = "65535" } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args new file mode 100644 index 0000000..4fe78a5 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args @@ -0,0 +1,12 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,\ +plaintext-channel=inputs,\ +image-compression=auto_glz,jpeg-wan-compression=auto,\ +zlib-glz-wan-compression=auto,\ +playback-compression=on,streaming-video=filter,disable-copy-paste,sasl -vga \ +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 \ +-device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml new file mode 100644 index 0000000..b22fbcc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml @@ -0,0 +1,45 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' defaultMode='secure'> + <listen type='address' address='127.0.0.1'/> + <channel name='main' mode='secure'/> + <channel name='inputs' mode='insecure'/> + <image compression='auto_glz'/> + <jpeg compression='auto'/> + <zlib compression='auto'/> + <playback compression='on'/> + <streaming mode='filter'/> + <clipboard copypaste='no'/> + </graphics> + <video> + <model type='qxl' ram='65536' vram='18432' heads='1'/> + </video> + <video> + <model type='qxl' ram='65536' vram='32768' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 060acf2..1cdb964 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -685,6 +685,12 @@ mymain(void) QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB, QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_USB_REDIR, QEMU_CAPS_CHARDEV_SPICEVMC); + driver.config->spiceSASL = 1; + DO_TEST("graphics-spice-sasl", + QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, + QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL); + driver.config->spiceSASL = 0; DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); -- 1.8.3.1

2 2

[libvirt] [PATCH] netcf: Don't complain when cleanup is called before init
by Christophe Fergeau 17 Oct '13

17 Oct '13

netcfStateInitialize() initializes the driverState variable, and when netcfStateCleanup is called, it will call virReportError() if driverState is NULL. This is not consistent with what other state objects are doing, they return -1 without reporting an error in such cases. See also https://www.redhat.com/archives/libvir-list/2013-October/msg00809.html: On Thu, Oct 17, 2013 at 01:40:19PM +0100, Daniel P. Berrange wrote: > We don't want virStateCleanup to skip execution if virStateInitialize > has failed though - every callback in virStateCleanup should be written > to be safe if its corresponding init function hasn't run. --- src/interface/interface_backend_netcf.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index fac059d..c4e18c4 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -100,11 +100,8 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED, static int netcfStateCleanup(void) { - if (!driverState) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Attempt to close netcf state driver already closed")); + if (!driverState) return -1; - } if (virObjectUnref(driverState)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 1.8.3.1

2 1

[libvirt] [PATCH] vircgroupmock.c: Avoid crashing if lstat() not found
by Michal Privoznik 17 Oct '13

17 Oct '13

In init_syms() we admit that stat() may not exists (in favor of existing __lxstat). However, if previously an library checked for existence of these two symbols and wisely has chosen the existing one, now, that we've mocked the both symbols, we are confusing the library. Moreover, such library will get SIGSEGV immediately after wrong decision, since reallstat is NULL. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/vircgroupmock.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index adc1718..0b44fda 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -542,10 +542,16 @@ int lstat(const char *path, struct stat *sb) errno = ENOMEM; return -1; } - ret = reallstat(newpath, sb); + if (reallstat) + ret = reallstat(newpath, sb); + else + ret = real__lxstat(_STAT_VER, newpath, sb); free(newpath); } else { - ret = reallstat(path, sb); + if (reallstat) + ret = reallstat(path, sb); + else + ret = real__lxstat(_STAT_VER, path, sb); } return ret; } -- 1.8.1.5

2 1

[libvirt] Migration issue php-libvirt
by Umar Draz 17 Oct '13

17 Oct '13

Hi All I am trying to migrate offline domain on other URI but its not working due to this error Failure!Libvirt last error: Requested operation is not valid: domain is not running I tried to use this option but not working VIR_MIGRATE_OFFLINE Please anybody help me? Br. Umar

4 8

[libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type
by Giuseppe Scrivano 17 Oct '13

17 Oct '13

Now each security model can define its own base label, that describes the default security context used by libvirt to run an hypervisor process. This information is exposed to users trough the host capabilities XML. *v3 major changes - support LXC - merge virSecurityDACSetUser and virSecurityDACSetGroup in virSecurityDACSetUserAndGroup - DAC sets the baselabel in virSecurityDACSetUserAndGroup - Use virDomainVirtTypeToString instead of hardcoding the name Giuseppe Scrivano (2): security: add new internal function "virSecurityManagerGetBaseLabel" capabilities: add baselabel per sec driver/virt type to secmodel docs/schemas/capability.rng | 8 ++++ src/conf/capabilities.c | 60 +++++++++++++++++++++++++++- src/conf/capabilities.h | 14 +++++++ src/libvirt_private.syms | 2 + src/lxc/lxc_conf.c | 10 ++++- src/qemu/qemu_conf.c | 21 ++++++++-- src/security/security_apparmor.c | 8 ++++ src/security/security_dac.c | 34 +++++++++++----- src/security/security_dac.h | 7 ++-- src/security/security_driver.h | 4 ++ src/security/security_manager.c | 22 +++++++++- src/security/security_manager.h | 2 + src/security/security_nop.c | 10 +++++ src/security/security_selinux.c | 12 ++++++ src/security/security_stack.c | 9 +++++ tests/capabilityschemadata/caps-qemu-kvm.xml | 2 + tests/capabilityschemadata/caps-test3.xml | 2 + 17 files changed, 204 insertions(+), 23 deletions(-) -- 1.8.3.1

2 9

[libvirt] PATCH: better error checking for LOCAL_PEERCRED
by Brian Candler 17 Oct '13

17 Oct '13

I was debugging libvirt with OSX today, and got as far as finding the problem with LOCAL_PEERCRED, then googled this only to find that Ryota Ozaki had fixed the problems a few days ago! However you still may find the following patch useful. It tightens up the checking in the LOCAL_PEERCRED block, and in particular fixes the unlocking of the socket in the error return path for invalid groups, by using the same logic from SO_PEERCRED - have a 'goto cleanup' in all return paths. (Detail: I found that when getsockopt was being called with SOL_SOCKET, cr_ngroups was typically <0, probably because it was uninitialised. However once the check for this was tightened, it hung because the socket wasn't being unlocked on return. So better to (a) initialise it to a negative value anyway, and (b) fix the return path) However I have not checked that NGROUPS is defined on other BSD-like systems. You could just have "if (cr.cr_ngroups <= 0)" instead. Regards, Brian Candler. --- src/rpc/virnetsocket.c.orig 2013-10-10 22:37:49.000000000 +0100 +++ src/rpc/virnetsocket.c 2013-10-12 22:51:57.000000000 +0100 @@ -1157,8 +1157,10 @@ { struct xucred cr; socklen_t cr_len = sizeof(cr); + int ret = -1; virObjectLock(sock); + cr.cr_ngroups = -1; # if defined(__APPLE__) if (getsockopt(sock->fd, SOL_LOCAL, LOCAL_PEERCRED, &cr, &cr_len) < 0) { # else @@ -1166,20 +1168,19 @@ # endif virReportSystemError(errno, "%s", _("Failed to get client socket identity")); - virObjectUnlock(sock); - return -1; + goto cleanup; } if (cr.cr_version != XUCRED_VERSION) { virReportError(VIR_ERR_SYSTEM_ERROR, "%s", _("Failed to get valid client socket identity")); - return -1; + goto cleanup; } - if (cr.cr_ngroups == 0) { + if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) { virReportError(VIR_ERR_SYSTEM_ERROR, "%s", _("Failed to get valid client socket identity groups")); - return -1; + goto cleanup; } /* PID and process creation time are not supported on BSDs */ @@ -1188,8 +1189,11 @@ *uid = cr.cr_uid; *gid = cr.cr_gid; + ret = 0; + +cleanup: virObjectUnlock(sock); - return 0; + return ret; } #else int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,

3 5

[libvirt] [PATCH 0/6] AArch64 support for libvirt.
by Pranavkumar Sawargaonkar 17 Oct '13

17 Oct '13

This patchset extends libvirt for AArch64 (armv8a). All patches have been tested on APM X-Gene SoC and we are able to run libvirtd on APM X-Gene SOC and spawn VMs remotely using virsh and virt-manager. Pranavkumar Sawargaonkar (6): AArch64: Add AArch64 architecture to list of valid arches. AArch64: CPU Support for AArch64 (ARMv8 64bit). AArch64: Parse cputopology from /proc/cpuinfo. Implement minimal sysinfo for AArch64 platforms. Add parsing of AArch64 qemu capabilities. AArch64: Add qemu capabilities schemeta for test. src/Makefile.am | 1 + src/cpu/cpu.c | 2 + src/cpu/cpu_aarch64.c | 79 ++++++++++++++++++++++++++ src/cpu/cpu_aarch64.h | 31 ++++++++++ src/nodeinfo.c | 5 +- src/qemu/qemu_capabilities.c | 4 ++ src/util/virarch.c | 1 + src/util/virarch.h | 1 + src/util/virsysinfo.c | 3 +- tests/capabilityschemadata/caps-qemu-kvm.xml | 11 ++++ tests/sysinfodata/aarch64cpuinfo.data | 10 ++++ tests/sysinfodata/aarch64sysinfo.expect | 10 ++++ tests/sysinfotest.c | 14 ++++- 13 files changed, 168 insertions(+), 4 deletions(-) create mode 100644 src/cpu/cpu_aarch64.c create mode 100644 src/cpu/cpu_aarch64.h create mode 100644 tests/sysinfodata/aarch64cpuinfo.data create mode 100644 tests/sysinfodata/aarch64sysinfo.expect -- 1.7.9.5

4 24

[libvirt] [PATCH]virsh: fix a typo in virsh-domain
by Chen Hanxiao 17 Oct '13

17 Oct '13

From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> s/it's/its Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6d241db..9017cac 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3036,7 +3036,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) &ctxt))) goto error; - /* tokenize the string from user and save it's parts into an array */ + /* tokenize the string from user and save its parts into an array */ if (vol_string && (nvol_list = vshStringToArray(vol_string, &vol_list)) < 0) goto error; -- 1.8.2.1

2 1

[libvirt] [PATCH] build: use the gnulib version of the .m4 files when present
by Giuseppe Scrivano 16 Oct '13

16 Oct '13

prevent aclocal to prefer .m4 files under m4/ to the version provided by gnulib. I have noticed this after './configure --help' gave me two different versions of "--enable-threads". This was caused by aclocal that preferred the version of lock.m4 provided by autopoint instead of using the newer version distributed with gnulib. Signed-off-by: Giuseppe Scrivano <gscrivan(a)redhat.com> --- bootstrap.conf | 5 ----- 1 file changed, 5 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 68c4a89..e7353e2 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -191,11 +191,6 @@ gnulib_tool_option_extras="\ " local_gl_dir=gnulib/local -# Convince bootstrap to use multiple m4 directories. -: ${ACLOCAL=aclocal} -ACLOCAL="$ACLOCAL -I m4" -export ACLOCAL - # Build prerequisites # Note that some of these programs are only required for 'make dist' to # succeed from a fresh git checkout; not all of these programs are -- 1.8.3.1

3 5

[libvirt] [PATCH] storage: document existing pools
by Eric Blake 16 Oct '13

16 Oct '13

We forgot to document several pool types. Furthermore, the RNG grammar did not allow arbitrary interleaving, which makes it harder than necessary to create a new pool from XML. * docs/formatstorage.html.in: Add docs for scsi, mpath, rbd, and sheepdog. * docs/schemas/storagepool.rng: Allow interleaving. * tests/storagepoolxml2xmlin/pool-sheepdog.xml: Test interleave. * tests/storagepoolxml2xmlin/pool-iscsi-auth.xml: Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- docs/formatstorage.html.in | 44 +-- docs/schemas/storagepool.rng | 356 ++++++++++++++----------- tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 12 +- tests/storagepoolxml2xmlin/pool-sheepdog.xml | 6 +- 4 files changed, 238 insertions(+), 180 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 3300d67..6f79b0c 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -18,9 +18,12 @@ The top level tag for a storage pool document is 'pool'. It has a single attribute <code>type</code>, which is one of <code>dir</code>, <code>fs</code>, <code>netfs</code>, <code>disk</code>, - <code>iscsi</code>, <code>logical</code>. This corresponds to the + <code>iscsi</code>, <code>logical</code>, <code>scsi</code> + (all <span class="since">since 0.4.1</span>), <code>mpath</code> + (<span class="since">since 0.7.1</span>), <code>rbd</code> + (<span class="since">since 0.9.13</span>), or <code>sheepdog</code> + (<span class="since">since 0.10.0</span>). This corresponds to the storage backend drivers listed further along in this document. - The storage pool XML format is available <span class="since">since 0.4.1</span> </p> <h3><a name="StoragePoolFirst">General metadata</a></h3> @@ -64,7 +67,8 @@ <p> A single <code>source</code> element is contained within the top level <code>pool</code> element. This tag is used to describe the source of - the storage pool. It can contain the following child elements: + the storage pool. The set of child elements that it will contain + depend on the pool type, but come from the following child elements: </p> <pre> @@ -89,17 +93,18 @@ <dl> <dt><code>device</code></dt> - <dd>Provides the source for pools backed by physical devices. + <dd>Provides the source for pools backed by physical devices + (fs, logical, disk, iscsi). May be repeated multiple times depending on backend driver. Contains a single attribute <code>path</code> which is the fully qualified path to the block device node. <span class="since">Since 0.4.1</span></dd> - <dt><code>directory</code></dt> - <dd>Provides the source for pools backed by directories. May + <dt><code>dir</code></dt> + <dd>Provides the source for pools backed by directories (dir). May only occur once. Contains a single attribute <code>path</code> which is the fully qualified path to the backing directory. <span class="since">Since 0.4.1</span></dd> <dt><code>adapter</code></dt> - <dd>Provides the source for pools backed by SCSI adapters. May + <dd>Provides the source for pools backed by SCSI adapters (scsi). May only occur once. Attribute <code>name</code> is the SCSI adapter name (ex. "scsi_host1". NB, although a name such as "host1" is still supported for backwards compatibility, it is not recommended). @@ -120,7 +125,8 @@ <span class="since">Since 0.6.2</span></dd> <dt><code>host</code></dt> <dd>Provides the source for pools backed by storage from a - remote server. Will be used in combination with a <code>directory</code> + remote server (netfs, iscsi, rbd, sheepdog). Will be + used in combination with a <code>directory</code> or <code>device</code> element. Contains an attribute <code>name</code> which is the hostname or IP address of the server. May optionally contain a <code>port</code> attribute for the protocol specific @@ -128,18 +134,19 @@ <dt><code>auth</code></dt> <dd>If present, the <code>auth</code> element provides the authentication credentials needed to access the source by the - setting of the <code>type</code> attribute. The <code>type</code> - must be either "chap" or "ceph". Additionally a mandatory attribute + setting of the <code>type</code> attribute (iscsi, + rbd). The <code>type</code> + must be either "chap" or "ceph". Use "ceph" for + Ceph RBD (Rados Block Device) network sources and use "iscsi" for CHAP + (Challenge-Handshake Authentication Protocol) iSCSI + targets. Additionally a mandatory attribute <code>username</code> identifies the username to use during authentication as well as a sub-element <code>secret</code> with a mandatory attribute <code>type</code>, to tie back to a <a href="formatsecret.html">libvirt secret object</a> that holds the actual password or other credentials. The domain XML intentionally does not expose the password, only the reference - to the object that manages the password. The secret element - <code>type</code> must be either "ceph" or "iscsi". Use "ceph" for - Ceph RBD (Rados Block Device) network sources and use "iscsi" for CHAP - (Challenge-Handshake Authentication Protocol) iSCSI targets. + to the object that manages the password. The <code>secret</code> element requires either a <code>uuid</code> attribute with the UUID of the secret object or a <code>usage</code> attribute matching the key that was specified in the @@ -148,11 +155,12 @@ </dd> <dt><code>name</code></dt> <dd>Provides the source for pools backed by storage from a - named element (e.g., a logical volume group name). + named element (logical, rbd, sheepdog). Contains a string identifier. <span class="since">Since 0.4.5</span></dd> <dt><code>format</code></dt> - <dd>Provides information about the format of the pool. This + <dd>Provides information about the format of the pool (fs, + netfs, disk, logical). This contains a single attribute <code>type</code> whose value is backend specific. This is typically used to indicate filesystem type, or network filesystem type, or partition table type, or @@ -174,7 +182,9 @@ <p> A single <code>target</code> element is contained within the top level - <code>pool</code> element. This tag is used to describe the mapping of + <code>pool</code> element for some types of pools (dir, fs, + netfs, logical, disk, iscsi, scsi, mpath). This tag is used to + describe the mapping of the storage pool into the host filesystem. It can contain the following child elements: </p> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index a4ef5af..66d3c22 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -29,192 +29,224 @@ <attribute name='type'> <value>dir</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcedir'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcedir'/> + <ref name='target'/> + </interleave> </define> <define name='poolfs'> <attribute name='type'> <value>fs</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcefs'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcefs'/> + <ref name='target'/> + </interleave> </define> <define name='poolnetfs'> <attribute name='type'> <value>netfs</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcenetfs'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcenetfs'/> + <ref name='target'/> + </interleave> </define> <define name='poollogical'> <attribute name='type'> <value>logical</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcelogical'/> - <ref name='targetlogical'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcelogical'/> + <ref name='targetlogical'/> + </interleave> </define> <define name='pooldisk'> <attribute name='type'> <value>disk</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcedisk'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcedisk'/> + <ref name='target'/> + </interleave> </define> <define name='pooliscsi'> <attribute name='type'> <value>iscsi</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourceiscsi'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourceiscsi'/> + <ref name='target'/> + </interleave> </define> <define name='poolscsi'> <attribute name='type'> <value>scsi</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcescsi'/> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcescsi'/> + <ref name='target'/> + </interleave> </define> <define name='poolmpath'> <attribute name='type'> <value>mpath</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <optional> - <ref name='sourcempath'/> - </optional> - <ref name='target'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <optional> + <ref name='sourcempath'/> + </optional> + <ref name='target'/> + </interleave> </define> <define name='poolrbd'> <attribute name='type'> <value>rbd</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcerbd'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcerbd'/> + </interleave> </define> <define name='poolsheepdog'> <attribute name='type'> <value>sheepdog</value> </attribute> - <ref name='commonmetadata'/> - <ref name='sizing'/> - <ref name='sourcesheepdog'/> + <interleave> + <ref name='commonmetadata'/> + <ref name='sizing'/> + <ref name='sourcesheepdog'/> + </interleave> </define> <define name='sourceinfovendor'> - <optional> - <element name='vendor'> - <attribute name='name'> - <text/> - </attribute> - </element> - </optional> - <optional> - <element name='product'> - <attribute name='name'> - <text/> - </attribute> - </element> - </optional> + <interleave> + <optional> + <element name='vendor'> + <attribute name='name'> + <text/> + </attribute> + </element> + </optional> + <optional> + <element name='product'> + <attribute name='name'> + <text/> + </attribute> + </element> + </optional> + </interleave> </define> <define name='commonmetadata'> - <element name='name'> - <ref name='genericName'/> - </element> - <optional> - <element name='uuid'> - <ref name='UUID'/> + <interleave> + <element name='name'> + <ref name='genericName'/> </element> - </optional> + <optional> + <element name='uuid'> + <ref name='UUID'/> + </element> + </optional> + </interleave> </define> <define name='sizing'> - <optional> - <element name='capacity'> - <ref name='scaledInteger'/> - </element> - </optional> - <optional> - <element name='allocation'> - <ref name='scaledInteger'/> - </element> - </optional> - <optional> - <element name='available'> - <ref name='scaledInteger'/> - </element> - </optional> + <interleave> + <optional> + <element name='capacity'> + <ref name='scaledInteger'/> + </element> + </optional> + <optional> + <element name='allocation'> + <ref name='scaledInteger'/> + </element> + </optional> + <optional> + <element name='available'> + <ref name='scaledInteger'/> + </element> + </optional> + </interleave> </define> <define name='permissions'> <optional> <element name='permissions'> - <element name='mode'> - <ref name='octalMode'/> - </element> - <element name='owner'> - <choice> - <ref name='unsignedInt'/> - <value>-1</value> - </choice> - </element> - <element name='group'> - <choice> - <ref name='unsignedInt'/> - <value>-1</value> - </choice> - </element> - <optional> - <element name='label'> - <text/> - </element> - </optional> + <interleave> + <element name='mode'> + <ref name='octalMode'/> + </element> + <element name='owner'> + <choice> + <ref name='unsignedInt'/> + <value>-1</value> + </choice> + </element> + <element name='group'> + <choice> + <ref name='unsignedInt'/> + <value>-1</value> + </choice> + </element> + <optional> + <element name='label'> + <text/> + </element> + </optional> + </interleave> </element> </optional> </define> <define name='target'> <element name='target'> - <element name='path'> - <ref name='absFilePath'/> - </element> - <ref name='permissions'/> + <interleave> + <element name='path'> + <ref name='absFilePath'/> + </element> + <ref name='permissions'/> + </interleave> </element> </define> <define name='targetlogical'> <element name='target'> - <optional> - <element name='path'> - <ref name='absFilePath'/> - </element> - </optional> - <ref name='permissions'/> + <interleave> + <optional> + <element name='path'> + <ref name='absFilePath'/> + </element> + </optional> + <ref name='permissions'/> + </interleave> </element> </define> @@ -413,74 +445,86 @@ <define name='sourcefs'> <element name='source'> - <ref name='sourceinfodev'/> - <ref name='sourcefmtfs'/> - <optional> - <ref name='sourceinfovendor'/> - </optional> + <interleave> + <ref name='sourceinfodev'/> + <ref name='sourcefmtfs'/> + <optional> + <ref name='sourceinfovendor'/> + </optional> + </interleave> </element> </define> <define name='sourcenetfs'> <element name='source'> - <ref name='sourceinfohost'/> - <ref name='sourceinfodir'/> - <ref name='sourcefmtnetfs'/> - <optional> - <ref name='sourceinfovendor'/> - </optional> + <interleave> + <ref name='sourceinfohost'/> + <ref name='sourceinfodir'/> + <ref name='sourcefmtnetfs'/> + <optional> + <ref name='sourceinfovendor'/> + </optional> + </interleave> </element> </define> <define name='sourcelogical'> <element name='source'> - <oneOrMore> + <interleave> + <oneOrMore> + <optional> + <ref name='sourceinfoname'/> + </optional> + <optional> + <ref name='sourceinfodev'/> + </optional> + </oneOrMore> + <ref name='sourcefmtlogical'/> <optional> - <ref name='sourceinfoname'/> - </optional> - <optional> - <ref name='sourceinfodev'/> + <ref name='sourceinfovendor'/> </optional> - </oneOrMore> - <ref name='sourcefmtlogical'/> - <optional> - <ref name='sourceinfovendor'/> - </optional> + </interleave> </element> </define> <define name='sourcedisk'> <element name='source'> - <ref name='sourceinfodev'/> - <ref name='sourcefmtdisk'/> - <optional> - <ref name='sourceinfovendor'/> - </optional> + <interleave> + <ref name='sourceinfodev'/> + <ref name='sourcefmtdisk'/> + <optional> + <ref name='sourceinfovendor'/> + </optional> + </interleave> </element> </define> <define name='sourceiscsi'> <element name='source'> - <ref name='sourceinfohost'/> - <ref name='sourceinfodev'/> - <optional> - <ref name='initiatorinfo'/> - </optional> - <optional> - <ref name='sourceinfoauth'/> - </optional> - <optional> - <ref name='sourceinfovendor'/> - </optional> + <interleave> + <ref name='sourceinfohost'/> + <ref name='sourceinfodev'/> + <optional> + <ref name='initiatorinfo'/> + </optional> + <optional> + <ref name='sourceinfoauth'/> + </optional> + <optional> + <ref name='sourceinfovendor'/> + </optional> + </interleave> </element> </define> <define name='sourcescsi'> <element name='source'> - <ref name='sourceinfoadapter'/> - <optional> - <ref name='sourceinfovendor'/> - </optional> + <interleave> + <ref name='sourceinfoadapter'/> + <optional> + <ref name='sourceinfovendor'/> + </optional> + </interleave> </element> </define> @@ -492,18 +536,22 @@ <define name='sourcerbd'> <element name='source'> - <ref name='sourceinfoname'/> - <ref name='sourceinfohost'/> - <optional> - <ref name='sourceinfoauth'/> - </optional> + <interleave> + <ref name='sourceinfoname'/> + <ref name='sourceinfohost'/> + <optional> + <ref name='sourceinfoauth'/> + </optional> + </interleave> </element> </define> <define name='sourcesheepdog'> <element name='source'> - <ref name='sourceinfohost'/> - <ref name='sourceinfoname'/> + <interleave> + <ref name='sourceinfohost'/> + <ref name='sourceinfoname'/> + </interleave> </element> </define> diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml b/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml index c81eb60..5e5d336 100644 --- a/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml +++ b/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml @@ -1,19 +1,19 @@ <pool type='iscsi'> - <name>virtimages</name> - <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid> <source> - <host name="iscsi.example.com"/> - <device path="demo-target"/> <auth type='chap' username='admin'> <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> </auth> + <device path="demo-target"/> + <host name="iscsi.example.com"/> </source> + <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid> <target> - <path>/dev/disk/by-path</path> <permissions> - <mode>0700</mode> <owner>0</owner> + <mode>0700</mode> <group>0</group> </permissions> + <path>/dev/disk/by-path</path> </target> + <name>virtimages</name> </pool> diff --git a/tests/storagepoolxml2xmlin/pool-sheepdog.xml b/tests/storagepoolxml2xmlin/pool-sheepdog.xml index 1287047..49b6baf 100644 --- a/tests/storagepoolxml2xmlin/pool-sheepdog.xml +++ b/tests/storagepoolxml2xmlin/pool-sheepdog.xml @@ -1,8 +1,8 @@ <pool type='sheepdog'> - <name>sheepdog</name> - <uuid>65fcba04-5b13-bd93-cff3-52ce48e11ad7</uuid> <source> - <host name='localhost' port='7000'/> <name>sheepdog</name> + <host name='localhost' port='7000'/> </source> + <uuid>65fcba04-5b13-bd93-cff3-52ce48e11ad7</uuid> + <name>sheepdog</name> </pool> -- 1.8.3.1

2 2

[libvirt] [PATCH] remote-driver: Fix 'leav' typo in comment
by Christophe Fergeau 16 Oct '13

16 Oct '13

--- Pushed under the trivial rule. src/remote/remote_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 96ccb99..87ef5a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3721,7 +3721,7 @@ static int remoteAuthCredSASL2Vir(int vircred) * * Build up the SASL callback structure. We register one callback for * each credential type that the libvirt client indicated they support. - * We explicitly leav the callback function pointer at NULL though, + * We explicitly leave the callback function pointer at NULL though, * because we don't actually want to get SASL callbacks triggered. * Instead, we want the start/step functions to return SASL_INTERACT. * This lets us give the libvirt client a list of all required -- 1.8.3.1

1 0

[libvirt] [PATCH v2] rpc: Correct the wrong payload size checking
by Osier Yang 16 Oct '13

16 Oct '13

<...> /* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX * and VIR_NET_MESSAGE_INITIAL. */ const VIR_NET_MESSAGE_LEN_MAX = 4; </...> However, msg->bufferLength includes the length word. The wrong checking was introduced by commit e914dcfd. * src/rpc/virnetmessage.c: - Correct the checking in virNetMessageEncodePayloadRaw - Use a new variable to track the new payload length in virNetMessageEncodePayloadRaw --- src/rpc/virnetmessage.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index 8f4e4bc..d60366b 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -346,15 +346,16 @@ int virNetMessageEncodePayload(virNetMessagePtr msg, /* Try to encode the payload. If the buffer is too small increase it. */ while (!(*filter)(&xdr, data)) { - if ((msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 > VIR_NET_MESSAGE_MAX) { + unsigned int newlen = (msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4; + + if (newlen > VIR_NET_MESSAGE_MAX) { virReportError(VIR_ERR_RPC, "%s", _("Unable to encode message payload")); goto error; } xdr_destroy(&xdr); - msg->bufferLength = (msg->bufferLength - VIR_NET_MESSAGE_LEN_MAX) * 4 + - VIR_NET_MESSAGE_LEN_MAX; + msg->bufferLength = newlen + VIR_NET_MESSAGE_LEN_MAX; if (VIR_REALLOC_N(msg->buffer, msg->bufferLength) < 0) goto error; @@ -426,10 +427,15 @@ int virNetMessageEncodePayloadRaw(virNetMessagePtr msg, /* If the message buffer is too small for the payload increase it accordingly. */ if ((msg->bufferLength - msg->bufferOffset) < len) { - if ((msg->bufferOffset + len) > VIR_NET_MESSAGE_MAX) { + if ((msg->bufferOffset + len) > + (VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX)) { virReportError(VIR_ERR_RPC, - _("Stream data too long to send (%zu bytes needed, %zu bytes available)"), - len, (VIR_NET_MESSAGE_MAX - msg->bufferOffset)); + _("Stream data too long to send " + "(%zu bytes needed, %zu bytes available)"), + len, + VIR_NET_MESSAGE_MAX + + VIR_NET_MESSAGE_LEN_MAX - + msg->bufferOffset); return -1; } -- 1.8.1.4

2 2

[libvirt] [PATCH] qemu: Fix the wrong logic of building cmd line for volume type disk
by Osier Yang 16 Oct '13

16 Oct '13

It builds a "libiscsi" uri if the volume type is "file" and "disk->auth.username" is specified, but I even see no reason to build the "libiscsi" uri for a "file" type volume, and the logic to check whether "disk->auth.username" is specified is also beyond understanding. It produces unexpected result like below: Disk config (pool is of 'fs' type): <disk type='volume' device='disk'> <driver name='qemu' type='raw'/> <auth username='myuser'> </auth> <source pool='default' volume='f18.img'/> <target dev='sda' bus='scsi'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> % virsh start f18 error: Failed to start domain f18 error: internal error: iscsi accepts only one host --- src/qemu/qemu_command.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abb62e9..9b6a27d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3806,13 +3806,7 @@ qemuBuildVolumeString(virConnectPtr conn, } break; case VIR_STORAGE_VOL_FILE: - if (disk->auth.username) { - if (qemuBuildISCSIString(conn, disk, opt) < 0) - goto cleanup; - virBufferAddChar(opt, ','); - } else { - virBufferEscape(opt, ',', ",", "file=%s,", disk->src); - } + virBufferEscape(opt, ',', ",", "file=%s,", disk->src); break; case VIR_STORAGE_VOL_NETWORK: /* Keep the compiler quite, qemuTranslateDiskSourcePool already -- 1.8.1.4

2 1

[libvirt] [PATCH v2] Introduce new OOM testing support
by Daniel P. Berrange 16 Oct '13

16 Oct '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The previous OOM testing support would re-run the entire "main" method each iteration, failing a different malloc each time. When a test suite has 'n' allocations, the number of repeats requires is (n * (n + 1) ) / 2. This gets very large, very quickly. This new OOM testing support instead integrates at the virtTestRun level, so each individual test case gets repeated, instead of the entire test suite. This means the values of 'n' are orders of magnitude smaller. The simple usage is $ VIR_TEST_OOM=1 ./qemuxml2argvtest ... 29) QEMU XML-2-ARGV clock-utc ... OK Test OOM for nalloc=36 .................................... OK 30) QEMU XML-2-ARGV clock-localtime ... OK Test OOM for nalloc=36 .................................... OK 31) QEMU XML-2-ARGV clock-france ... OK Test OOM for nalloc=38 ...................................... OK ... the second lines reports how many mallocs have to be failed, and thus how many repeats of the test will be run. If it crashes, then running under valgrind will often show the problem $ VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest When debugging problems it is also helpful to select an individual test case $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest When things get really tricky, it is possible to request that just specific allocs are failed. eg to fail allocs 5 -> 12, use $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-12 ../run valgrind ./qemuxml2argvtest In the worse case, you might want to know the stack trace of the alloc which was failed then VIR_TEST_OOM_TRACE can be set. If it is set to 1 then it will only print if it thinks a mistake happened. This is often not reliable, so setting it to 2 will make it print the stack trace for every alloc that is failed. $ VIR_TEST_OOM_TRACE=2 VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-5 ../run valgrind ./qemuxml2argvtest 30) QEMU XML-2-ARGV clock-localtime ... OK Test OOM for nalloc=36 !virAllocN /home/berrange/src/virt/libvirt/src/util/viralloc.c:180 virHashCreateFull /home/berrange/src/virt/libvirt/src/util/virhash.c:144 virDomainDefParseXML /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11745 virDomainDefParseNode /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12646 virDomainDefParse /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12590 testCompareXMLToArgvFiles /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:106 virtTestRun /home/berrange/src/virt/libvirt/tests/testutils.c:250 mymain /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:418 (discriminator 2) virtTestMain /home/berrange/src/virt/libvirt/tests/testutils.c:750 ?? ??:0 _start ??:? FAILED Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- docs/internals/oomtesting.html.in | 213 ++++++++++++++++++++++++++++++++++++++ docs/sitemap.html.in | 4 + tests/qemuargv2xmltest.c | 14 +-- tests/qemuxml2argvtest.c | 12 ++- tests/qemuxmlnstest.c | 18 ++-- tests/testutils.c | 195 +++++++++++++++++++++++++++++++++- tests/testutils.h | 2 + 7 files changed, 437 insertions(+), 21 deletions(-) create mode 100644 docs/internals/oomtesting.html.in diff --git a/docs/internals/oomtesting.html.in b/docs/internals/oomtesting.html.in new file mode 100644 index 0000000..652aff6 --- /dev/null +++ b/docs/internals/oomtesting.html.in @@ -0,0 +1,213 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> + <body> + <h1>Out of memory testing</h1> + + <ul id="toc"></ul> + + + <p> + This page describes how to use the test suite todo out of memory + testing. + </p> + + <h2>Building with OOM testing</h2> + + <p> + Since OOM testing requires hooking into the malloc APIs, it is + not enabled by default. The flag <code>--enable-test-oom</code> + must be given to <code>configure</code>. When this is done the + libvirt allocation APIs will have some hooks enabled. + </p> + + <pre> +$ ./configure --enable-test-oom +</pre> + + + <h2><a name="basicoom">Basic OOM testing support</a></h2> + + <p> + The first step in validating OOM usage is to run a test suite + with full OOM testing enabled. This is done by setting the + <code>VIR_TEST_OOM=1</code> environment variable. The way this + works is that it runs the test once normally to "prime" any + static memory allocations. Then it runs it once more counting + the total number of memory allocations. Then it runs it in a + loop failing a different memory allocation each time. For every + memory allocation failure triggered, it expects the test case + to return an error. OOM testing is quite slow requiring each + test case to be executed O(n) times, where 'n' is the total + number of memory allocations. This results in a total number + of memory allocations of '(n * (n + 1) ) / 2' + </p> + + <pre> +$ VIR_TEST_OOM=1 ./qemuxml2argvtest + 1) QEMU XML-2-ARGV minimal ... OK + Test OOM for nalloc=42 .......................................... OK + 2) QEMU XML-2-ARGV minimal-s390 ... OK + Test OOM for nalloc=28 ............................ OK + 3) QEMU XML-2-ARGV machine-aliases1 ... OK + Test OOM for nalloc=38 ...................................... OK + 4) QEMU XML-2-ARGV machine-aliases2 ... OK + Test OOM for nalloc=38 ...................................... OK + 5) QEMU XML-2-ARGV machine-core-on ... OK + Test OOM for nalloc=37 ..................................... OK +...snip... +</pre> + + <p> + In this output, the first line shows the normal execution and + the test number, and the second line shows the total number + of memory allocations from that test case. + </p> + + <h3><a name="valgrind">Tracking failures with valgrind</a></h3> + + <p> + The test suite should obviously *not* crash during OOM testing. + If it does crash, then to assist in tracking down the problem + it is worth using valgrind and only running a single test case. + For example, supposing test case 5 crashed. Then re-run the + test with + </p> + + <pre> +$ VIR_TEST_OOM=1 VIR_TEST_RANGE=5 ../run valgrind ./qemuxml2argvtest +...snip... + 5) QEMU XML-2-ARGV machine-core-on ... OK + Test OOM for nalloc=37 ..................................... OK +...snip... + </pre> + + <p> + Valgrind should report the cause of the crash - for example a + double free or use of uninitialized memory or NULL pointer + access. + </p> + + <h3><a name="stacktraces">Tracking failures with stack traces</a></h3> + + <p> + With some really difficult bugs valgrind is not sufficient to + identify the cause. In this case, it is useful to identify the + precise allocation which was failed, to allow the code path + to the error to be traced. The <code>VIR_TEST_OOM</code> + env variable can be given a range of memory allocations to + test. So if a test case has 150 allocations, it can be told + to only test allocation numbers 7-10. The <code>VIR_TEST_OOM_TRACE</code> + variable can be used to print out stack traces. + </p> + + <pre> +$ VIR_TEST_OOM_TRACE=2 VIR_TEST_OOM=1:7-10 VIR_TEST_RANGE=5 \ + ../run valgrind ./qemuxml2argvtest + 5) QEMU XML-2-ARGV machine-core-on ... OK + Test OOM for nalloc=37 !virAllocN +/home/berrange/src/virt/libvirt/src/util/viralloc.c:180 +virDomainDefParseXML +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11786 (discriminator 1) +virDomainDefParseNode +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677 +virDomainDefParse +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621 +testCompareXMLToArgvFiles +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107 +virtTestRun +/home/berrange/src/virt/libvirt/tests/testutils.c:266 +mymain +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2) +virtTestMain +/home/berrange/src/virt/libvirt/tests/testutils.c:791 +__libc_start_main +??:? +_start +??:? +!virAlloc +/home/berrange/src/virt/libvirt/src/util/viralloc.c:133 +virDomainDiskDefParseXML +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:4790 +virDomainDefParseXML +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11797 +virDomainDefParseNode +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677 +virDomainDefParse +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621 +testCompareXMLToArgvFiles +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107 +virtTestRun +/home/berrange/src/virt/libvirt/tests/testutils.c:266 +mymain +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2) +virtTestMain +/home/berrange/src/virt/libvirt/tests/testutils.c:791 +__libc_start_main +??:? +_start +??:? +!virAllocN +/home/berrange/src/virt/libvirt/src/util/viralloc.c:180 +virXPathNodeSet +/home/berrange/src/virt/libvirt/src/util/virxml.c:609 +virDomainDefParseXML +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11805 +virDomainDefParseNode +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677 +virDomainDefParse +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621 +testCompareXMLToArgvFiles +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107 +virtTestRun +/home/berrange/src/virt/libvirt/tests/testutils.c:266 +mymain +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2) +virtTestMain +/home/berrange/src/virt/libvirt/tests/testutils.c:791 +__libc_start_main +??:? +_start +??:? +!virAllocN +/home/berrange/src/virt/libvirt/src/util/viralloc.c:180 +virDomainDefParseXML +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11808 (discriminator 1) +virDomainDefParseNode +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677 +virDomainDefParse +/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621 +testCompareXMLToArgvFiles +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107 +virtTestRun +/home/berrange/src/virt/libvirt/tests/testutils.c:266 +mymain +/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2) +virtTestMain +/home/berrange/src/virt/libvirt/tests/testutils.c:791 +__libc_start_main +??:? +_start +??:? + </pre> + + <h3><a name="noncrash">Non-crash related problems</a></h3> + + <p> + Not all memory allocation bugs result in code crashing. Sometimes + the code will be silently ignoring the allocation failure, resulting + in incorrect data being produced. For example the XML parser may + mistakenly treat an allocation failure as indicating that an XML + attribute was not set in the input document. It is hard to identify + these problems from the test suite automatically. For this, the + test suites should be run with <code>VIR_TEST_DEBUG=1</code> set + and then stderr analysed for any unexpected data. For example, + the XML conversion may show an embedded "(null)" literal, or the + test suite might complain about missing elements / attributes + in the actual vs expected data. These are all signs of bugs in + OOM handling. In the future the OOM tests will be enhanced to + validate that an error VIR_ERR_NO_MEMORY is returned for each + allocation failed, rather than some other error. + </p> + </body> +</html> diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index d821a9e..d885f02 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -338,6 +338,10 @@ <a href="internals/locking.html">Lock managers</a> <span>Use lock managers to protect disk content</span> </li> + <li> + <a href="internals/oomtesting.html">Out of memory testing</a> + <span>Simulating OOM conditions in the test suite</span> + </li> </ul> </li> <li> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 6dd8bb0..92fc89a 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -42,7 +42,6 @@ static int testCompareXMLToArgvFiles(const char *xml, char *cmd = NULL; int ret = -1; virDomainDefPtr vmdef = NULL; - char *log; if (virtTestLoadFile(cmdfile, &cmd) < 0) goto fail; @@ -53,13 +52,16 @@ static int testCompareXMLToArgvFiles(const char *xml, cmd, NULL, NULL, NULL))) goto fail; - if ((log = virtTestLogContentAndReset()) == NULL) - goto fail; - if ((*log != '\0') != expect_warning) { + if (!virtTestOOMActive()) { + char *log; + if ((log = virtTestLogContentAndReset()) == NULL) + goto fail; + if ((*log != '\0') != expect_warning) { + VIR_FREE(log); + goto fail; + } VIR_FREE(log); - goto fail; } - VIR_FREE(log); if (!(actualxml = virDomainDefFormat(vmdef, 0))) goto fail; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 060acf2..83ddbad 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -107,7 +107,8 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, VIR_DOMAIN_XML_INACTIVE))) { - if (flags & FLAG_EXPECT_PARSE_ERROR) + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_PARSE_ERROR)) goto ok; goto out; } @@ -170,7 +171,8 @@ static int testCompareXMLToArgvFiles(const char *xml, migrateFrom, migrateFd, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &testCallbacks))) { - if (flags & FLAG_EXPECT_FAILURE) { + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_FAILURE)) { ret = 0; if (virTestGetDebug() > 1) fprintf(stderr, "Got expected error: %s\n", @@ -184,7 +186,8 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (!!virGetLastError() != !!(flags & FLAG_EXPECT_ERROR)) { + if (!virtTestOOMActive() && + (!!virGetLastError() != !!(flags & FLAG_EXPECT_ERROR))) { if (virTestGetDebug() && (log = virtTestLogContentAndReset())) fprintf(stderr, "\n%s", log); goto out; @@ -205,7 +208,8 @@ static int testCompareXMLToArgvFiles(const char *xml, } ok: - if (flags & FLAG_EXPECT_ERROR) { + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_ERROR)) { /* need to suppress the errors */ virResetLastError(); } diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 2cc15d1..38c98bd 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -117,15 +117,17 @@ static int testCompareXMLToArgvFiles(const char *xml, &testCallbacks))) goto fail; - if (!!virGetLastError() != expectError) { - if (virTestGetDebug() && (log = virtTestLogContentAndReset())) - fprintf(stderr, "\n%s", log); - goto fail; - } + if (!virtTestOOMActive()) { + if (!!virGetLastError() != expectError) { + if (virTestGetDebug() && (log = virtTestLogContentAndReset())) + fprintf(stderr, "\n%s", log); + goto fail; + } - if (expectError) { - /* need to suppress the errors */ - virResetLastError(); + if (expectError) { + /* need to suppress the errors */ + virResetLastError(); + } } if (!(actualargv = virCommandToString(cmd))) diff --git a/tests/testutils.c b/tests/testutils.c index 5d634b4..bc1e34e 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -47,6 +47,13 @@ #include "virprocess.h" #include "virstring.h" +#ifdef TEST_OOM +# ifdef TEST_OOM_TRACE +#include <dlfcn.h> +#include <execinfo.h> +# endif +#endif + #ifdef HAVE_PATHS_H # include <paths.h> #endif @@ -64,6 +71,18 @@ static unsigned int testDebug = -1; static unsigned int testVerbose = -1; static unsigned int testExpensive = -1; +#ifdef TEST_OOM +static unsigned int testOOM = 0; +static unsigned int testOOMStart = -1; +static unsigned int testOOMEnd = -1; +static unsigned int testOOMTrace = 0; +# ifdef TEST_OOM_TRACE +void *testAllocStack[30]; +int ntestAllocStack; +# endif +#endif +static bool testOOMActive = false; + static size_t testCounter = 0; static size_t testStart = 0; static size_t testEnd = 0; @@ -71,6 +90,19 @@ static size_t testEnd = 0; char *progname; char *abs_srcdir; +bool virtTestOOMActive(void) +{ + return testOOMActive; +} + +#ifdef TEST_OOM_TRACE +static void virTestAllocHook(int nalloc ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + ntestAllocStack = backtrace(testAllocStack, ARRAY_CARDINALITY(testAllocStack)); +} +#endif + void virtTestResult(const char *name, int ret, const char *msg, ...) { va_list vargs; @@ -109,6 +141,35 @@ void virtTestResult(const char *name, int ret, const char *msg, ...) va_end(vargs); } +#ifdef TEST_OOM_TRACE +static void +virTestShowTrace(void) +{ + size_t j; + for (j = 2; j < ntestAllocStack; j++) { + Dl_info info; + char *cmd; + + dladdr(testAllocStack[j], &info); + if (info.dli_fname && + strstr(info.dli_fname, ".so")) { + if (virAsprintf(&cmd, "addr2line -f -e %s %p", + info.dli_fname, + ((void*)((unsigned long long)testAllocStack[j] + - (unsigned long long)info.dli_fbase))) < 0) + continue; + } else { + if (virAsprintf(&cmd, "addr2line -f -e %s %p", + (char*)(info.dli_fname ? info.dli_fname : "<unknown>"), + testAllocStack[j]) < 0) + continue; + } + ignore_value(system(cmd)); + VIR_FREE(cmd); + } +} +#endif + /* * Runs test * @@ -155,7 +216,7 @@ virtTestRun(const char *title, !((testCounter-1) % 40)) { fprintf(stderr, " %-3zu\n", (testCounter-1)); fprintf(stderr, " "); - } + } if (ret == 0) fprintf(stderr, "."); else if (ret == EXIT_AM_SKIP) @@ -164,6 +225,77 @@ virtTestRun(const char *title, fprintf(stderr, "!"); } +#ifdef TEST_OOM + if (testOOM && ret != EXIT_AM_SKIP) { + int nalloc; + int oomret; + int start, end; + size_t i; + virResetLastError(); + virAllocTestInit(); +# ifdef TEST_OOM_TRACE + virAllocTestHook(virTestAllocHook, NULL); +# endif + oomret = body(data); + nalloc = virAllocTestCount(); + fprintf(stderr, " Test OOM for nalloc=%d ", nalloc); + if (testOOMStart == -1 || + testOOMEnd == -1) { + start = 0; + end = nalloc; + } else { + start = testOOMStart; + end = testOOMEnd + 1; + } + testOOMActive = true; + for (i = start; i < end; i++) { + bool missingFail = false; +# ifdef TEST_OOM_TRACE + memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack)); + ntestAllocStack = 0; +# endif + virAllocTestOOM(i + 1, 1); + oomret = body(data); + + /* fprintf() disabled because XML parsing APIs don't allow + * distinguish between element / attribute not present + * in the XML (which is non-fatal), vs OOM / malformed + * which should be fatal. Thus error reporting for + * optionally present XML is mostly broken. + */ + if (oomret == 0) { + missingFail = true; +# if 0 + fprintf(stderr, " alloc %zu failed but no err status\n", i + 1); +# endif + } else { + virErrorPtr lerr = virGetLastError(); + if (!lerr) { +# if 0 + fprintf(stderr, " alloc %zu failed but no error report\n", i + 1); +# endif + missingFail = true; + } + } + if ((missingFail && testOOMTrace) || (testOOMTrace > 1)) { + fprintf(stderr, "%s", "!"); +# ifdef TEST_OOM_TRACE + virTestShowTrace(); +# endif + ret = -1; + } else { + fprintf(stderr, "%s", "."); + } + } + testOOMActive = false; + if (ret == 0) + fprintf(stderr, " OK\n"); + else + fprintf(stderr, " FAILED\n"); + virAllocTestInit(); + } +#endif /* TEST_OOM */ + return ret; } @@ -192,7 +324,6 @@ virtTestLoadFile(const char *file, char **buf) tmplen = buflen = st.st_size + 1; if (VIR_ALLOC_N(*buf, buflen) < 0) { - fprintf(stderr, "%s: larger than available memory (> %d)\n", file, buflen); VIR_FORCE_FCLOSE(fp); return -1; } @@ -468,7 +599,8 @@ virtTestLogOutput(virLogSource source ATTRIBUTE_UNUSED, { struct virtTestLogData *log = data; virCheckFlags(VIR_LOG_STACK_TRACE,); - virBufferAsprintf(&log->buf, "%s: %s", timestamp, str); + if (!testOOMActive) + virBufferAsprintf(&log->buf, "%s: %s", timestamp, str); } static void @@ -537,6 +669,9 @@ int virtTestMain(int argc, int ret; bool abs_srcdir_cleanup = false; char *testRange = NULL; +#ifdef TEST_OOM + char *oomstr; +#endif abs_srcdir = getenv("abs_srcdir"); if (!abs_srcdir) { @@ -597,6 +732,60 @@ int virtTestMain(int argc, } } +#ifdef TEST_OOM + if ((oomstr = getenv("VIR_TEST_OOM")) != NULL) { + char *next; + if (testDebug == -1) + testDebug = 1; + testOOM = 1; + if (oomstr[0] != '\0' && + oomstr[1] == ':') { + if (virStrToLong_ui(oomstr + 2, &next, 10, &testOOMStart) < 0) { + fprintf(stderr, "Cannot parse range %s\n", oomstr); + return EXIT_FAILURE; + } + if (*next == '\0') { + testOOMEnd = testOOMStart; + } else { + if (*next != '-') { + fprintf(stderr, "Cannot parse range %s\n", oomstr); + return EXIT_FAILURE; + } + if (virStrToLong_ui(next+1, NULL, 10, &testOOMEnd) < 0) { + fprintf(stderr, "Cannot parse range %s\n", oomstr); + return EXIT_FAILURE; + } + } + } else { + testOOMStart = -1; + testOOMEnd = -1; + } + } + +# ifdef TEST_OOM_TRACE + if ((oomstr = getenv("VIR_TEST_OOM_TRACE")) != NULL) { + if (virStrToLong_ui(oomstr, NULL, 10, &testOOMTrace) < 0) { + fprintf(stderr, "Cannot parse oom trace %s\n", oomstr); + return EXIT_FAILURE; + } + } +# else + if (getenv("VIR_TEST_OOM_TRACE")) { + fprintf(stderr, "%s", "OOM test tracing not enabled in this build\n"); + return EXIT_FAILURE; + } +# endif +#else /* TEST_OOM */ + if (getenv("VIR_TEST_OOM")) { + fprintf(stderr, "%s", "OOM testing not enabled in this build\n"); + return EXIT_FAILURE; + } + if (getenv("VIR_TEST_OOM_TRACE")) { + fprintf(stderr, "%s", "OOM test tracing not enabled in this build\n"); + return EXIT_FAILURE; + } +#endif /* TEST_OOM */ + ret = (func)(); if (abs_srcdir_cleanup) diff --git a/tests/testutils.h b/tests/testutils.h index 478b53c..e6f494d 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -40,6 +40,8 @@ extern char *progname; extern char *abs_srcdir; +bool virtTestOOMActive(void); + void virtTestResult(const char *name, int ret, const char *msg, ...) ATTRIBUTE_FMT_PRINTF(3,4); int virtTestRun(const char *title, -- 1.8.3.1

1 0

[libvirt] [PATCH 0/6] Try to fix selinux/securityfs mount handling in LXC
by Daniel P. Berrange 16 Oct '13

16 Oct '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> This series tries to address the problem of LXC containers failing to start because 'securityfs' is not compiled in. In doing this some general cleanup was performed on the LXC mount code. Daniel P. Berrange (6): Add virFileIsMountPoint function Remove unused 'opts' field from LXC basic mounts struct Remove pointless 'srcpath' variable in lxcContainerMountBasicFS Remove duplicate entries in lxcBasicMounts array Add flag to lxcBasicMounts to control use in user namespaces Skip any files which are not mounted on the host src/libvirt_private.syms | 1 + src/lxc/lxc_container.c | 86 +++++++++++++++++++++++++++++------------------- src/util/virfile.c | 58 ++++++++++++++++++++++++++++++++ src/util/virfile.h | 2 ++ 4 files changed, 113 insertions(+), 34 deletions(-) -- 1.8.3.1

4 16

[libvirt] [PATCHv2 00/23] Better approach to add the "pvticketlocks" feature
by Peter Krempa 16 Oct '13

16 Oct '13

This version resolves conflicts with Eric's const correctnes series. The paravirtual spinlock feature was introduced recently into the qemu and linux code base. To test it you need the most recent -rc version of the linux kernel and the git version of qemu. As the requirements to activate this feature in the guest are really volatile (kernel support, qemu support .. .) I decided to improve the original version so that it detects if the feature was actually enabled in the guest. This avoids possible problems with breaking guest ABI when migrating between two hosts with different kernels. Additionally we may start enforcin other features too. This patchset is partialy based on Jiri's work on getting actual cpu definition from a running qemu. Jiri Denemark (8): cpu: Add support for loading and storing CPU data cpu: x86: Rename struct cpuX86cpuid as virCPUx86CPUID cpu: x86: Rename struct cpuX86Data as virCPUx86Data cpu: x86: Rename x86DataFree() as virCPUx86DataFree() cpu: x86: Rename x86MakeCPUData as virCPUx86MakeData cpu: x86: Rename x86DataAddCpuid as virCPUx86DataAddCPUID cpu: Export few x86-specific APIs qemu: Add monitor APIs to fetch CPUID data from QEMU Peter Krempa (15): schema: Rename option to make it reusable conf: Clean up few error messages qemu: command: Fix macro indentation cpu: x86: Rename data_iterator and DATA_ITERATOR_INIT cpu: x86: Fix return types of x86cpuidMatch and x86cpuidMatchMasked cpu: x86: Fix function header formatting and whitespace cpu: x86: Use whitespace to clarify context and use consistent labels cpu: x86: Clean up error messages in x86VendorLoad() qemu: Clean up check of maximum cpu count supported by a machine type cpu_x86: Refactor storage of CPUID data to add support for KVM features cpu: x86: Parse the CPU feature map only once cpu: x86: Add internal CPUID features support and KVM feature bits conf: Refactor storing and usage of feature flags qemu: Add support for paravirtual spinlocks in the guest qemu: process: Validate specific CPUID flags of a guest docs/formatdomain.html.in | 7 + docs/schemas/domaincommon.rng | 18 +- src/conf/domain_conf.c | 200 ++++-- src/conf/domain_conf.h | 3 +- src/cpu/cpu.c | 41 ++ src/cpu/cpu.h | 16 +- src/cpu/cpu_x86.c | 699 ++++++++++++--------- src/cpu/cpu_x86.h | 9 + src/cpu/cpu_x86_data.h | 26 +- src/libvirt_private.syms | 8 + src/libxl/libxl_conf.c | 9 +- src/lxc/lxc_container.c | 6 +- src/qemu/qemu_command.c | 31 +- src/qemu/qemu_monitor.c | 21 + src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 107 ++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 50 +- src/vbox/vbox_tmpl.c | 45 +- src/xenapi/xenapi_driver.c | 10 +- src/xenapi/xenapi_utils.c | 22 +- src/xenxs/xen_sxpr.c | 20 +- src/xenxs/xen_xm.c | 30 +- tests/Makefile.am | 1 + .../qemumonitorjson-getcpu-empty.data | 2 + .../qemumonitorjson-getcpu-empty.json | 46 ++ .../qemumonitorjson-getcpu-filtered.data | 4 + .../qemumonitorjson-getcpu-filtered.json | 46 ++ .../qemumonitorjson-getcpu-full.data | 5 + .../qemumonitorjson-getcpu-full.json | 46 ++ .../qemumonitorjson-getcpu-host.data | 6 + .../qemumonitorjson-getcpu-host.json | 45 ++ tests/qemumonitorjsontest.c | 76 +++ .../qemuxml2argv-pv-spinlock-disabled.args | 5 + .../qemuxml2argv-pv-spinlock-disabled.xml | 26 + .../qemuxml2argv-pv-spinlock-enabled.args | 5 + .../qemuxml2argv-pv-spinlock-enabled.xml | 26 + tests/qemuxml2argvtest.c | 2 + tests/qemuxml2xmltest.c | 2 + 39 files changed, 1279 insertions(+), 448 deletions(-) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-empty.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-filtered.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml -- 1.8.3.2

3 50

[libvirt] [PATCH] virprocess: make virProcessGetStartTime work on Mac OS X
by Ryota Ozaki 16 Oct '13

16 Oct '13

This patch addresses the following warning output by libvirtd: warning : virProcessGetStartTime:843 : Process start time of pid XXXXX not available on this platform For Mac OS X, we can use similar instructions to FreeBSD to get the start time of a process. The difference between them is struct kinfo_proc; kinfo_proc.ki_start is the start time of a process for FreeBSD while kinfo_proc.kp_proc.p_starttime for Mac OS X. Note that this patch works for Lion and Mountain Lion, however, doesn't work for Snow Leopard for some reason; sysctlnametomib("kern.proc.pid", ...) fails on Snow Leopard with the following error: error : virProcessGetStartTime:822 : Unable to get MIB of kern.proc.pid: No such file or directory This is unexpected. man 3 sysctl of Snow Leopard says it should work... Anyway libvirtd is able to launch on Snow Leopard regardless of the error. Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com> --- src/util/virprocess.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 9fc3207..5a0ed0d 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -32,7 +32,7 @@ #endif #include <sched.h> -#ifdef __FreeBSD__ +#if defined(__FreeBSD__) || defined(__APPLE__) # include <sys/param.h> # include <sys/sysctl.h> # include <sys/user.h> @@ -809,7 +809,7 @@ cleanup: VIR_FREE(buf); return ret; } -#elif defined(__FreeBSD__) +#elif defined(__FreeBSD__) || defined(__APPLE__) int virProcessGetStartTime(pid_t pid, unsigned long long *timestamp) { @@ -817,7 +817,12 @@ int virProcessGetStartTime(pid_t pid, int mib[4]; size_t len = 4; - sysctlnametomib("kern.proc.pid", mib, &len); + /* FIXME: It doesn't work on Snow Leopard for some reason */ + if (sysctlnametomib("kern.proc.pid", mib, &len) < 0) { + virReportSystemError(errno, "%s", + _("Unable to get MIB of kern.proc.pid")); + return -1; + } len = sizeof(struct kinfo_proc); mib[3] = pid; @@ -828,7 +833,11 @@ int virProcessGetStartTime(pid_t pid, return -1; } +# if defined(__FreeBSD__) *timestamp = (unsigned long long)p.ki_start.tv_sec; +# else + *timestamp = (unsigned long long)p.kp_proc.p_starttime.tv_sec; +# endif return 0; -- 1.8.4

2 2

[libvirt] [PATCH 0/3] leave cgroup value checking to kernel
by Chen Hanxiao 16 Oct '13

16 Oct '13

From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> Since 2.6.39, kernel changed the minimum weight of device blkio while libvirt hard-coded the value range checking. So we should leave the job of checking to kernel. Chen Hanxiao (3): [libvirt]docs: change the minimum weight description for blkio [libvirt]cgroup: show error when EINVAL caught [libvirt]cgroup: leave value checking to kernel docs/formatdomain.html.in | 6 ++++-- src/util/vircgroup.c | 26 +++++++++++--------------- tools/virsh.pod | 11 ++++++----- 3 files changed, 21 insertions(+), 22 deletions(-) -- 1.8.2.1

2 7

[libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
by Chen Hanxiao 16 Oct '13

16 Oct '13

From: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> If we enable userns, the process with uid/gid in idmap should have enough permission to access dir we provided for containers. Currently, the debug log is very implicit or misleading sometimes. This patch will help clarify this for us when using debug log or virsh. v2: syntax-check clean v3: reliable method for checking permission of dir Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com> --- src/lxc/lxc_container.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8abaea0..9a05e30 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -110,6 +110,13 @@ struct __lxc_child_argv { int handshakefd; }; +typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t; +struct __lxc_userns_DirPermCheck_argv { + uid_t uid; + gid_t gid; + virDomainDefPtr vmDef; +}; + static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, const char *srcprefix); @@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def) return false; } +static +int lxcContainerCheckDirPermissionChild(void *argv) +{ + size_t i; + lxc_userns_DirPermCheck_argv_t *args = argv; + uid_t uid = args->uid; + uid_t gid = args->gid; + virDomainDefPtr vmDef = args->vmDef; + char *path; + + if (virSetUIDGID(uid, gid, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("setuid or setgid failed")); + _exit(-1); + } + + for (i = 0; i < vmDef->nfss; i++) { + path = vmDef->fss[i]->src; + if (access(path, R_OK) || access(path, W_OK) || virFileIsExecutable(path)) { + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d", + vmDef->fss[i]->src, uid, gid); + _exit(-1); + } + } + + _exit(0); +} + +/* + * Helper function for helping check + * whether we have enough privilege + * to operate the source dir when userns enabled + * @vmDef: pointer to vm definition structure + * Returns 0 on success or -1 in case of error + */ +static int +lxcContainerCheckDirPermission(virDomainDefPtr vmDef) +{ + uid_t uid; + gid_t gid; + int cpid = 0; + int status; + char *childStack; + char *stack; + int flags = SIGCHLD; + + uid = vmDef->idmap.uidmap[0].target; + gid = vmDef->idmap.gidmap[0].target; + + lxc_userns_DirPermCheck_argv_t args = { + .uid = uid, + .gid = gid, + .vmDef = vmDef + }; + + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0) + return -1; + + childStack = stack + (getpagesize() * 4); + cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, &args); + VIR_FREE(stack); + if (cpid < 0) { + virReportSystemError(errno, "%s", + _("Unable to clone to check permission of directory")); + return -1; + } else if (virProcessWait(cpid, &status) < 0) { + return -1; + } + + if (WEXITSTATUS(status) != 0) { + virReportSystemError(errno, "%s", + _("Check the permission of source dir provided for container")); + return -1; + } + + return 0; +} + /** * lxcContainerStart: * @def: pointer to virtual machine structure @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def, if (userns_supported()) { VIR_DEBUG("Enable user namespace"); cflags |= CLONE_NEWUSER; + if (lxcContainerCheckDirPermission(def) < 0) { + return -1; + } } else { virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); -- 1.8.2.1

2 4

Re: [libvirt] [Qemu-devel] [RFC PATCH] spapr-vty: workaround "reg" property for old kernels
by Anthony Liguori 15 Oct '13

15 Oct '13

On Tue, Oct 15, 2013 at 3:47 PM, Alexey Kardashevskiy <aik(a)ozlabs.ru> wrote: > On 10/16/2013 02:03 AM, Alexander Graf wrote: >> On 10/15/2013 10:50 AM, Alexey Kardashevskiy wrote: >>> Old kernels (< 3.1) handle hvcX devices different in different parts. >>> Sometime the kernel assumes that the hvc device numbers start from zero >>> and if there is just one hvc, then it is hvc0. >>> >>> However kernel's add_preferred_console() uses the very last byte of >>> the VTY's "reg" property as an hvc number so it might end up with something >>> different than hvc. >>> >>> The problem appears on SLES11SP3 and RHEL6. If to run QEMU without >>> -nodefaults, then the default VTY is created first on a VIO bus and gets >>> reg==0x71000000 so it will be hvc0 and everything will be fine. >>> If to run QEMU with: >>> -nodefaults \ >>> -chardev "socket,id=char1,host=localhost,port=8001,server,telnet,mux=on" \ >>> -device "spapr-vty,chardev=char1" \ >>> -mon "chardev=char1,mode=readline,id=mon1" \ >>> >>> then the exactly the same config is expected but in this case spapr-vty >>> gets reg==0x71000001 and therefore it becomes hvc1 and lots of debug >>> output is missing. SLES11SP3 does not even boot as /dev/console is >>> redirected to /dev/hvc0 which is dead. >>> >>> The issue can be solved by manual selection of VTY's "reg" property to >>> have last byte equal to zero. >>> >>> The alternative would be to use separate "reg" property counter for >>> automatic "reg" property generation and this is what the patch does. >>> >>> Signed-off-by: Alexey Kardashevskiy<aik(a)ozlabs.ru> >>> --- >>> >>> Since libvirt uses "-nodefault" a lot and in this case "spapr-nvram" gets >>> created first and gets reg=0x71000000, we cannot just ignore this. Also, >>> it does not seem an option to require libvirt users to specify spapr-vty >>> "reg" property every time. >>> >>> Can anyone think of a simpler solutionu? Thanks. >>> >>> >>> --- >>> hw/ppc/spapr_vio.c | 7 ++++++- >>> include/hw/ppc/spapr_vio.h | 1 + >>> 2 files changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c >>> index a6a0a51..2d56950 100644 >>> --- a/hw/ppc/spapr_vio.c >>> +++ b/hw/ppc/spapr_vio.c >>> @@ -438,7 +438,11 @@ static int spapr_vio_busdev_init(DeviceState *qdev) >>> VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, >>> dev->qdev.parent_bus); >>> >>> do { >>> - dev->reg = bus->next_reg++; >>> + if (!object_dynamic_cast(OBJECT(qdev), "spapr-vty")) { >>> + dev->reg = bus->next_reg++; >>> + } else { >>> + dev->reg = bus->next_vty_reg++; >>> + } >>> } while (reg_conflict(dev)); >>> } >>> >>> @@ -501,6 +505,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void) >>> qbus = qbus_create(TYPE_SPAPR_VIO_BUS, dev, "spapr-vio"); >>> bus = DO_UPCAST(VIOsPAPRBus, bus, qbus); >>> bus->next_reg = 0x71000000; >>> + bus->next_vty_reg = 0x71000100; >> >> This breaks as soon as you pass in more than 0x100 devices that are non-vty >> into the guest, no? > > Will we ever have this much? Ah, anyway, this code already checks if the > address is taken and fails if it is. And there is still a possibility to > assign addresses manually. > >> The reg property really describes the virtual slot a device is in. > > We use 0x71000000. I saw xmls from libvirt where VTY's reg is 0x30000000. > Whether it is a slot or not, QEMU/SLOF/Kernel does not seem to care about > absolute value :) > >> Couldn't >> we do that allocation explicitly and push it from libvirt, just like we do >> it with the slots for PCI? Yes, this is the only solution. We make no promises with respect to argument ordering. libvirt needs to explicitly specify reg values to create a stable device tree (just like it does with PCI). Regards, Anthony Liguori > > That is the other possibility, yes. But in this case "-nodefaults" must not > create spapr-nvram automatically and if we do that, we'll break existing > setups. > > >> >> >> Alex >> >> >>> >>> /* hcall-vio */ >>> spapr_register_hypercall(H_VIO_SIGNAL, h_vio_signal); >>> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h >>> index d8b3b03..3a92d9e 100644 >>> --- a/include/hw/ppc/spapr_vio.h >>> +++ b/include/hw/ppc/spapr_vio.h >>> @@ -73,6 +73,7 @@ struct VIOsPAPRDevice { >>> struct VIOsPAPRBus { >>> BusState bus; >>> uint32_t next_reg; >>> + uint32_t next_vty_reg; >>> int (*init)(VIOsPAPRDevice *dev); >>> int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off); >>> }; >> > > > -- > Alexey >

1 0