May 2013 - Devel - libvirt List Archives

[libvirt] [PATCH v2] qemuOpenVhostNet: Decrease vhostfdSize on open failure
by Michal Privoznik 29 May '13

29 May '13

Currently, if there's an error opening /dev/vhost-net (e.g. because it doesn't exist) but it's not required we proceed with vhostfd array filled with -1 and vhostfdSize unchanged. Later, when constructing the qemu command line only non-negative items within vhostfd array are taken into account. This means, vhostfdSize may be greater than the actual count of non-negative items in vhostfd array. This results in improper command line arguments being generated, e.g.: -netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null) --- src/qemu/qemu_command.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0373626..c4a162a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -486,6 +486,10 @@ qemuOpenVhostNet(virDomainDefPtr def, "but is unavailable")); goto error; } + VIR_WARN("Unable to open vhost-net. Opened so far %d, requested %d", + i, *vhostfdSize); + *vhostfdSize = i; + break; } } virDomainAuditNetDevice(def, net, "/dev/vhost-net", *vhostfdSize); @@ -6560,12 +6564,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } for (i = 0; i < vhostfdSize; i++) { - if (vhostfd[i] >= 0) { - virCommandTransferFD(cmd, vhostfd[i]); - if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) { - virReportOOMError(); - goto cleanup; - } + virCommandTransferFD(cmd, vhostfd[i]); + if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) { + virReportOOMError(); + goto cleanup; } } -- 1.8.2.1

2 2

[libvirt] [PATCH] Introduce startupPolicy for chardev to make guest OS bootable when hardware failure happens in log disk
by Seiji Aguchi 28 May '13

28 May '13

[Problem] Currently, guest OS's messages can be logged to a local disk of host OS by creating chadevs with options below. -chardev file,id=charserial0,path=<log file's path> -device isa-serial,chardev=chardevserial0,id=serial0 When a hardware failure happens in the disk, qemu-kvm can't create the chardevs. In this case, guest OS doesn't boot up. Actually, there are users who don't desire that guest OS goes down due to a hardware failure of a log disk only.Therefore, qemu should offer some way to boot guest OS up even if the log disk is broken. [Solution] This patch supports startupPolicy for chardev. The starupPolicy is introduced just in case where chardev is "file" because this patch aims for making guest OS boot up when a hardware failure happens. In other cases ,pty, dev, pipe and unix, it is not introduced because they don't access to hardware. The policy works as follows. - If the value is "optional", guestOS boots up by dropping the chardev. - If other values are specified, guestOS fails to boot up. (the default) Description about original startupPolicy attribute: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=e5a84d74a2789a917bf394… Signed-off-by: Seiji Aguchi <seiji.aguchi(a)hds.com> --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_process.c | 25 ++++++++++++++++++++++++- tests/virt-aa-helper-test | 3 +++ 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f325c3c..1e1bf27 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4044,13 +4044,20 @@ qemu-kvm -net nic,model=? /dev/null <p> A file is opened and all data sent to the character device is written to the file. + It is possible to define policy whether guestOS boots up + if the file is not accessible. This is done by a startupPolicy + attribute: + <ul> + <li>If the vaule is "optional", guestOS boots up by dropping the file.</li> + <li>If other values are specified, guestOS fails to boot up. (the default)</li> + </ul> </p> <pre> ... <devices> <serial type="file"> - <source path="/var/log/vm/vm-serial.log"/> + <source path="/var/log/vm/vm-serial.log" startupPolicy="optional"/> <target port="1"/> </serial> </devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..6fc0a3c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2706,6 +2706,9 @@ </optional> <optional> <attribute name="path"/> + <optional> + <ref name='startupPolicy'/> + </optional> </optional> <optional> <attribute name="host"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a8b5dfd..6680f15 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6467,6 +6467,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *path = NULL; char *mode = NULL; char *protocol = NULL; + char *startupPolicy = NULL; int remaining = 0; while (cur != NULL) { @@ -6487,6 +6488,9 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, !(flags & VIR_DOMAIN_XML_INACTIVE))) path = virXMLPropString(cur, "path"); + if (startupPolicy == NULL && + def->type == VIR_DOMAIN_CHR_TYPE_FILE) + startupPolicy = virXMLPropString(cur, "startupPolicy"); break; case VIR_DOMAIN_CHR_TYPE_UDP: @@ -6559,6 +6563,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.file.path = path; path = NULL; + + def->data.file.startupPolicy = + virDomainStartupPolicyTypeFromString(startupPolicy); + startupPolicy = NULL; break; case VIR_DOMAIN_CHR_TYPE_STDIO: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a0f23a..e709951 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1052,6 +1052,7 @@ struct _virDomainChrSourceDef { /* no <source> for null, vc, stdio */ struct { char *path; + int startupPolicy; /* enum virDomainStartupPolicy */ } file; /* pty, file, pipe, or device */ struct { char *host; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e75c8c9..6e2f78e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2442,7 +2442,30 @@ qemuProcessPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, virReportSystemError(errno, _("Unable to pre-create chardev file '%s'"), dev->source.data.file.path); - return -1; + if (dev->source.data.file.startupPolicy != + VIR_DOMAIN_STARTUP_POLICY_OPTIONAL) { + return -1; + } + VIR_FREE(dev->source.data.file.path); + /* + * Change a destination to /dev/null to boot guest OS up + * even if a log disk is broken. + */ + VIR_WARN("Switch the destination to /dev/null"); + dev->source.data.file.path = strdup("/dev/null"); + + if (!(dev->source.data.file.path)) { + virReportOOMError(); + return -1; + } + + if ((fd = open(dev->source.data.file.path, + O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) { + virReportSystemError(errno, + _("Unable to pre-create chardev file '%s'"), + dev->source.data.file.path); + return -1; + } } VIR_FORCE_CLOSE(fd); diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test index af91c61..7172fd6 100755 --- a/tests/virt-aa-helper-test +++ b/tests/virt-aa-helper-test @@ -255,6 +255,9 @@ testme "0" "disk (empty cdrom)" "-r -u $valid_uuid" "$test_xml" sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial" "-r -u $valid_uuid" "$test_xml" +sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='file'><source path='$tmpdir/serial.log' startupPolicy='optional'/><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" +testme "0" "serial" "-r -u $valid_uuid" "$test_xml" + sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e "s,</devices>,<serial type='pty'><target port='0'/></serial></devices>,g" "$template_xml" > "$test_xml" testme "0" "serial (pty)" "-r -u $valid_uuid" "$test_xml" -- 1.7.1

2 6

[libvirt] [PATCH] build: fix build with older gcc
by Eric Blake 28 May '13

28 May '13

gcc 4.1.2 (hello, RHEL 5!) fails to build on 32-bit platforms with: conf/domain_conf.c: In function 'virDomainDefParseXML': conf/domain_conf.c:10581: warning: integer constant is too large for 'long' type * src/conf/domain_conf.c (virDomainDefParseXML): Mark large constants. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..2b4e160 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10578,7 +10578,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (def->cputune.quota > 0 && (def->cputune.quota < 1000 || - def->cputune.quota > 18446744073709551)) { + def->cputune.quota > 18446744073709551LL)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Value of cputune quota must be in range " "[1000, 18446744073709551]")); @@ -10610,7 +10610,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (def->cputune.emulator_quota > 0 && (def->cputune.emulator_quota < 1000 || - def->cputune.emulator_quota > 18446744073709551)) { + def->cputune.emulator_quota > 18446744073709551LL)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Value of cputune emulator_quota must be in range " "[1000, 18446744073709551]")); -- 1.8.1.4

1 1

[libvirt] [PATCH] build: drop unused variable
by Eric Blake 28 May '13

28 May '13

Compilation for mingw failed: ../../src/util/virutil.c: In function 'virGetWin32DirectoryRoot': ../../src/util/virutil.c:1094:9: error: unused variable 'ret' [-Werror=unused-variable] * src/util/virutil.c (virGetWin32DirectoryRoot): Silence compiler warning. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Compilation still fails in the tests directory, due to the use of fprintf(stderr, "%zu", size_t); I'm still debating whether I can get it fixed in gnulib and take the risk of a gnulib backport before rc2, or whether we'll have to come up with some other less-invasive workaround. src/util/virutil.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 028f1d1..a29da14 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1091,7 +1091,6 @@ static int virGetWin32DirectoryRoot(char **path) { char windowsdir[MAX_PATH]; - int ret = 0; *path = NULL; -- 1.8.1.4

1 1

[libvirt] [PATCH 0/2]write separate module for hostdev passthrough
by Chunyan Liu 28 May '13

28 May '13

This patch sets include two patches: 1/2 is the implementation of the hostdev passthrough common library. To meet two purposes: a. move qemu hostdev APIs to common library so that it could be used by all hypervisor drivers. b. maintain a global hostdev in-use state. 2/2 is the implementation of pci passthrough to libxl driver. There should be more patches to change qemu driver and lxc driver codes to switch to common library APIs, but I think it's better to have you review the common library first. After common library APIs confirmed, then modify qemu/lxc driver codes in a time. The implementation to libxl driver (2/2) is an example of using common library. Please review, thanks! Chunyan Liu (2): add pci passthrough common library add pci passthrough impl to libxl po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt.c | 5 + src/libvirt_private.syms | 15 + src/libxl/libxl_conf.c | 44 ++ src/libxl/libxl_driver.c | 21 +- src/util/virhostdevmanager.c | 1218 ++++++++++++++++++++++++++++++++++++++++++ src/util/virhostdevmanager.h | 91 ++++ src/util/virpci.c | 17 +- src/util/virpci.h | 7 +- src/util/virusb.c | 19 +- src/util/virusb.h | 4 +- 12 files changed, 1426 insertions(+), 17 deletions(-) create mode 100644 src/util/virhostdevmanager.c create mode 100644 src/util/virhostdevmanager.h

4 8

[libvirt] [PATCH] syntax: fix broken error message in previous patch
by Eric Blake 28 May '13

28 May '13

Osier Yang pointed out that I introduced a syntax error in my syntax check (I really shouldn't make last-minute changes without testing them....). /bin/sh: -c: line 2: syntax error near unexpected token `;' /bin/sh: -c: line 2: ` { echo 'maint.mk: incorrect whitespace, see HACKING for rules' 2>&; \' make: *** [bracket-spacing-check] Error 1 * cfg.mk (bracket-spacing-check): Fix copy-and-paste error. (sc_prohibit_duplicate_header): Use consistent spelling. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. cfg.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfg.mk b/cfg.mk index a01df56..4ffa020 100644 --- a/cfg.mk +++ b/cfg.mk @@ -779,7 +779,7 @@ sc_prohibit_duplicate_header: }' $$i || fail=1; \ done; \ if test $$fail -eq 1; then \ - { echo "$(ME)": avoid duplicate headers >&2; exit 1; } \ + { echo '$(ME): avoid duplicate headers' 1>&2; exit 1; } \ fi; # Don't include "libvirt/*.h" in "" form. @@ -845,7 +845,7 @@ syntax-check: $(top_srcdir)/HACKING bracket-spacing-check bracket-spacing-check: $(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \ $(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \ - { echo "$(ME): incorrect whitespace, see HACKING for rules" 2>&; \ + { echo '$(ME): incorrect whitespace, see HACKING for rules' 1>&2; \ exit 1; } # sc_po_check can fail if generated files are not built first -- 1.8.1.4

1 0

[libvirt] [PATCH 0/2] More semicolon cleanups
by Eric Blake 28 May '13

28 May '13

In response to: https://www.redhat.com/archives/libvir-list/2013-May/msg01791.html Eric Blake (2): syntax: prefer space after semicolon in for loop syntax-check: mandate space after mid-line semicolon HACKING | 23 +++++++++++++++++++++++ build-aux/bracket-spacing.pl | 12 ++++++++++++ cfg.mk | 2 +- docs/hacking.html.in | 29 +++++++++++++++++++++++++++++ examples/domsuspend/suspend.c | 2 +- python/libvirt-override.c | 38 +++++++++++++++++++------------------- src/conf/interface_conf.c | 14 +++++++------- src/security/virt-aa-helper.c | 2 +- src/util/virconf.c | 4 ++-- src/util/virhook.c | 2 +- src/util/virlog.c | 10 +++++----- src/util/virsocketaddr.c | 12 ++++++------ src/util/virsysinfo.c | 4 ++-- src/util/viruuid.c | 4 ++-- src/vbox/vbox_tmpl.c | 11 ++++++++--- src/xen/xen_hypervisor.c | 3 ++- tools/virsh-domain-monitor.c | 6 +++--- tools/virsh-domain.c | 6 +++--- 18 files changed, 127 insertions(+), 57 deletions(-) -- 1.8.1.4

3 7

[libvirt] [PATCH] qemu: Don't report error on successful media eject
by Cole Robinson 28 May '13

28 May '13

If we are just ejecting media, ret == -1 even after the retry loop determines that the tray is open, as requested. This means media disconnect always report's error. Fix it, and fix some other mini issues: - Don't overwrite the 'eject' error message if the retry loop fails - Move the retries decrement inside the loop, otherwise the final loop might succeed, yet retries == 0 and we will raise error - Setting ret = -1 in the disk->src check is unneeded - Fix comment typos cc: mprivozn(a)redhat.com --- src/qemu/qemu_hotplug.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1ed61db..3b277ef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -98,10 +98,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ - while (retries--) { + while (retries) { if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) break; + retries--; virObjectUnlock(vm); VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d", retries); usleep(500 * 1000); /* sleep 500ms */ @@ -109,19 +110,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } virObjectUnref(vm); - if (disk->src) { - /* deliberately don't depend on 'ret' as 'eject' may have failed for the - * fist time and we are gonna check the drive state anyway */ - const char *format = NULL; - - /* We haven't succeeded yet */ - ret = -1; - - if (retries <= 0) { + if (retries <= 0) { + if (ret == 0) { + /* If ret == -1, EjectMedia already set an error message */ virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("Unable to eject media before changing it")); - goto audit; + _("Unable to eject media")); } + goto audit; + } + ret = 0; + + if (disk->src) { + /* deliberately don't depend on 'ret' as 'eject' may have failed the + * first time and we are gonna check the drive state anyway */ + const char *format = NULL; if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) { if (disk->format > 0) -- 1.8.2.1

2 2

[libvirt] [libvirt-tck PATCH] 121-block-info.t: omit network
by Guido Günther 28 May '13

28 May '13

qemu:///session doesn't have a default network so we fail with: ./scripts/domain/121-block-info.t .. 1/29 # Defining transient storage pool # Generic guest with pervious created vol ./scripts/domain/121-block-info.t .. 12/29 # Failed test 'Create domain' # at /var/lib/jenkins/jobs/libvirt-tck-build/workspace/lib//Sys/Virt/TCK.pm line 803. # expected Sys::Virt::Domain object # found 'libvirt error code: 43, message: Network not found: no network with matching name 'default' # ' The network isn't needed so simply leave it out. --- scripts/domain/121-block-info.t | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/domain/121-block-info.t b/scripts/domain/121-block-info.t index a25d075..dad00c6 100644 --- a/scripts/domain/121-block-info.t +++ b/scripts/domain/121-block-info.t @@ -91,7 +91,6 @@ $guest->rmdisk(); $guest->disk(format => { name => "qemu", type => $disktype }, type => "file", src => $path, dst => $dst); $guest->disk(format => { name => "qemu", type => $disktype }, type => "file", src=> $path2, dst => $dst2); $guest->disk(format => { name => "qemu", type => "qcow2" }, type => "file", src=> $path3, dst => $dst3); -$guest->interface(type => "network", source => "default", model => "virtio", mac => "52:54:00:22:22:22"); $xml = $guest->as_xml; my $dom; -- 1.7.10.4

2 2

[libvirt] snapshot-create-as Permission denied
by yue 28 May '13

28 May '13

May 25 20:50:59 ovirtdev NetworkManager[2370]: <warn> /sys/devices/virtual/net/vnet0: couldn't determine device driver; ignoring... root@ovirtdev images]# virsh snapshot-create-as linux snapshot1 "snapshot1" --disk-only --atomic error: internal error unable to execute QEMU command 'transaction': /var/lib/libvirt/images/test.snapshot1: error while creating qcow2: Permission denied

3 4