- Devel - libvirt List Archives

[libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator
by Matthias Bolte 24 Aug '11

24 Aug '11

--- src/Makefile.am | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8fe7120..5ba189c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -812,8 +812,9 @@ endif BUILT_SOURCES += $(ESX_DRIVER_GENERATED) -$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input $(srcdir)/esx/esx_vi_generator.py - $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py +$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \ + $(srcdir)/esx/esx_vi_generator.py + $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py if WITH_ESX if WITH_DRIVER_MODULES -- 1.7.4.1

2 2

[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
by Adam Litke 24 Aug '11

24 Aug '11

From: Osier Yang <jyang(a)redhat.com> * src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error. * src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed. Without this patch, libvirt always report: An error occurred, but the cause is unknown This patch was adapted from a patch by Osier Yang <jyang(a)redhat.com> to break out detection of unrecognized text monitor commands into a separate function. Signed-off-by: Adam Litke <agl(a)us.ibm.com> --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + } if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..9119f06 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,20 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); } +/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version")) + return 1; + return 0; +} + static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, @@ -3031,18 +3045,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, char *cmd = NULL; char *reply = NULL; int ret; - - if (mode == BLOCK_JOB_ABORT) - ret = virAsprintf(&cmd, "block_job_cancel %s", device); - else if (mode == BLOCK_JOB_INFO) - ret = virAsprintf(&cmd, "info block-jobs"); - else if (mode == BLOCK_JOB_SPEED) - ret = virAsprintf(&cmd, "block_job_set_speed %s %llu", device, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "info block-jobs"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + ret = virAsprintf(&cmd, "%s %s %llu", cmd_name, device, bandwidth * 1024ULL * 1024ULL); - else if (mode == BLOCK_JOB_PULL) - ret = virAsprintf(&cmd, "block_stream %s", device); - else + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else { return -1; + } if (ret < 0) { virReportOOMError(); @@ -3056,6 +3076,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + if (qemuMonitorTextCommandNotFound(reply)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.3

2 3

[libvirt] [RFC 00/12] add support for new USB2 emulation / EHCI device
by Marc-André Lureau 24 Aug '11

24 Aug '11

Hi, The following patch intents to implement support for new USB2 and USB redirection added in QEMU 0.15. It follows somewhat Daniel's plan send earlier on this ML: https://www.redhat.com/archives/libvir-list/2011-August/msg00816.html There are some parts that I don't really know what they should be like: the "autoassign" or the default values at the domain XML level. Related RHBZ https://bugzilla.redhat.com/show_bug.cgi?id=725670 cheers Marc-André Lureau (12): Add various USB devices QEMU_CAPS Add USB controller models Add a new controller type 'usb' with optionnal 'model' USB controller can have a PCI address child element USB devices gain a new USB address child element Add USB companion controllers support Add USB hub device Modify USB port to be defined as a port path RFC: Don't reserve slot 1 if a USB controller is defined there RFC: Don't append 0 at usb id, so that it is compatible with legacy -usb Add a usb1 & usb2 qemuxml2argv test Add usb-redir device docs/formatdomain.html.in | 79 +++++- docs/schemas/domain.rng | 124 ++++++-- src/conf/domain_conf.c | 333 +++++++++++++++++++- src/conf/domain_conf.h | 59 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 34 ++ src/qemu/qemu_capabilities.h | 11 + src/qemu/qemu_command.c | 262 ++++++++++++++-- src/qemu/qemu_command.h | 10 +- src/qemu/qemu_hotplug.c | 12 +- src/vmx/vmx.c | 12 +- tests/qemuhelptest.c | 16 +- .../qemuxml2argv-input-usbmouse-addr.args | 1 + .../qemuxml2argv-input-usbmouse-addr.xml | 27 ++ .../qemuxml2argv-usb-controller.args | 1 + .../qemuxml2argv-usb-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml | 19 ++ .../qemuxml2argv-usb-ich9-companion.args | 6 + .../qemuxml2argv-usb-ich9-companion.xml | 30 ++ .../qemuxml2argv-usb-ich9-ehci-addr.args | 1 + .../qemuxml2argv-usb-ich9-ehci-addr.xml | 18 + .../qemuxml2argv-usb-piix3-controller.args | 1 + .../qemuxml2argv-usb-piix3-controller.xml | 16 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml | 31 ++ tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 8 + tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 33 ++ tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 74 +++++ tests/qemuxml2argvtest.c | 34 ++ 31 files changed, 1196 insertions(+), 77 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-hub.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ports.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml -- 1.7.6

4 21

[libvirt] [PATCH] esx: Refactor a repeated string in the generator
by Matthias Bolte 23 Aug '11

23 Aug '11

--- src/esx/esx_vi_generator.py | 24 +++++++++++++----------- 1 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 239ec73..8a128df 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1817,17 +1817,19 @@ for obj in managed_objects_by_name.values(): -types_typedef.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -types_typeenum.write("/* Generated by esx_vi_generator.py */\n\n") -types_typetostring.write("/* Generated by esx_vi_generator.py */\n\n") -types_typefromstring.write("/* Generated by esx_vi_generator.py */\n\n") -types_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -types_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -methods_macro.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -helpers_header.write("/* Generated by esx_vi_generator.py */\n\n\n\n") -helpers_source.write("/* Generated by esx_vi_generator.py */\n\n\n\n") +notice = "/* Generated by esx_vi_generator.py */\n\n\n\n" + +types_typedef.write(notice) +types_typeenum.write(notice) +types_typetostring.write(notice) +types_typefromstring.write(notice) +types_header.write(notice) +types_source.write(notice) +methods_header.write(notice) +methods_source.write(notice) +methods_macro.write(notice) +helpers_header.write(notice) +helpers_source.write(notice) -- 1.7.4.1

2 2

[libvirt] Curl/HTTP block device
by Radek Hladik 23 Aug '11

23 Aug '11

Hi, I've noticed newer version of curl in last qemu changelog. It seems that its needed for curl block device. Unfortunatelly, I was not able to find out more. I tried to look into qemu source code and to some patches but I am not much wiser. As I understand it, qemu could be able to use a curl compatible URL as block device. Probably only in readonly mode but it still would be very usefull for i.e. ISO images. Can anyone point me in the right direction and more importantly, is this feature available via Libvirt? I checked the documentation on libvirt.org and it seems that remode disk can be only nbd/rbd/sheepdog. Radek

3 3

[libvirt] [PATCH] maint: fix comment typos
by Eric Blake 23 Aug '11

23 Aug '11

* src/qemu/qemu_driver.c (qemuDomainSaveInternal): Fix typo. * src/conf/domain_event.c (virDomainEventDispatchMatchCallback): Likewise. * daemon/libvirtd.c (daemonRunStateInit): Likewise. * src/lxc/lxc_container.c (lxcContainerChildMountSort): Likewise. * src/util/virterror.c (virCopyError, virRaiseErrorFull): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxprSound): Likewise. --- Pushing under the trivial rule. daemon/libvirtd.c | 2 +- src/conf/domain_event.c | 4 ++-- src/lxc/lxc_container.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/util/virterror.c | 4 ++-- src/xenxs/xen_sxpr.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 0530ba5..5969a82 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1131,7 +1131,7 @@ static void daemonRunStateInit(void *opaque) virNetServerPtr srv = opaque; /* Start the stateful HV drivers - * This is delibrately done after telling the parent process + * This is deliberately done after telling the parent process * we're ready, since it can take a long time and this will * seriously delay OS bootup process */ if (virStateInitialize(virNetServerIsPrivileged(srv)) < 0) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2e0524b..3189346 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1103,11 +1103,11 @@ static int virDomainEventDispatchMatchCallback(virDomainEventPtr event, return 0; if (cb->dom) { - /* Delibrately ignoring 'id' for matching, since that + /* Deliberately ignoring 'id' for matching, since that * will cause problems when a domain switches between * running & shutoff states & ignoring 'name' since * Xen sometimes renames guests during migration, thus - * leaving 'uuid' as the only truely reliable ID we can use*/ + * leaving 'uuid' as the only truly reliable ID we can use*/ if (memcmp(event->dom.uuid, cb->dom->uuid, VIR_UUID_BUFLEN) == 0) return 1; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 895ef09..e425328 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -287,7 +287,7 @@ static int lxcContainerChildMountSort(const void *a, const void *b) const char **sa = (const char**)a; const char **sb = (const char**)b; - /* Delibrately reversed args - we need to unmount deepest + /* Deliberately reversed args - we need to unmount deepest children first */ return strcmp(*sb, *sa); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 035ddb4..2c677b0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2564,7 +2564,7 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, /* Avoid throwing an error here, since it is possible * that with NFS we can't actually stat() the file. * The subsequent codepaths will still raise an error - * if a truely fatal problem is hit */ + * if a truly fatal problem is hit */ is_reg = true; } else { is_reg = !!S_ISREG(sb.st_mode); diff --git a/src/util/virterror.c b/src/util/virterror.c index 9a27feb..c5babb1 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -248,7 +248,7 @@ virCopyError(virErrorPtr from, to->int1 = from->int1; to->int2 = from->int2; /* - * Delibrately not setting 'conn', 'dom', 'net' references + * Deliberately not setting 'conn', 'dom', 'net' references */ return ret; } @@ -708,7 +708,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, * Save the information about the error */ /* - * Delibrately not setting conn, dom & net fields since + * Deliberately not setting conn, dom & net fields since * they're utterly unsafe */ to->domain = domain; diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index 7b292bc..5e278ae 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -604,7 +604,7 @@ xenParseSxprSound(virDomainDefPtr def, * Special compatability code for Xen with a bogus * sound=all in config. * - * NB delibrately, don't include all possible + * NB deliberately, don't include all possible * sound models anymore, just the 2 that were * historically present in Xen's QEMU. * -- 1.7.4.4

1 0

[libvirt] [PATCH v4] Add support for fd: protocol
by Corey Bryant 23 Aug '11

23 Aug '11

sVirt provides SELinux MAC isolation for Qemu guest processes and their corresponding resources (image files). sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, which is needed for image file isolation when using the sVirt SELinux security driver in libvirt. The proposed solution entails a combination of Qemu, libvirt, and SELinux patches that work together to isolate multiple guests' images when they're stored in the same NFS mount. This results in an environment where sVirt isolation and NFS image file isolation can both be provided. Currently, Qemu opens an image file in addition to performing the necessary read and write operations. The proposed solution will move the open out of Qemu and into libvirt. Once libvirt opens an image file for the guest, it will pass the file descriptor to Qemu via a new fd: protocol. If the image file resides in an NFS mount, the following SELinux policy changes will provide image isolation: - A new SELinux boolean is created (e.g. virt_read_write_nfs) to allow Qemu (svirt_t) to only have SELinux read and write permissions on nfs_t files - Qemu (svirt_t) also gets SELinux use permissions on libvirt (virtd_t) file descriptors Following is a sample invocation of Qemu using the fd: protocol on the command line: qemu -drive file=fd:4,format=qcow2 The fd: protocol is also supported by the drive_add monitor command. This requires that the specified file descriptor is passed to the monitor alongside a prior getfd monitor command. This patch also supports the following features for the fd: protocol: - -snapshot command line option - savevm monitor command This patch does not contain support for the following features, all of which are planned to be supported in the future: - Copy-on-write backing files - snapshot_blkdev monitor command - -cdrom command line option - -drive command line option with media=cdrom - change monitor command v2: - Add drive_add monitor command support - Fence off unsupported features that re-open image file v3: - Fence off cdrom and change monitor command support v4: - Removed checks that fenced off features for fd: protocol - Enabled -snapshot and savevm support Signed-off-by: Corey Bryant <coreyb(a)linux.vnet.ibm.com> --- block.c | 9 ++--- block/raw-posix.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++---- migration-fd.c | 2 +- monitor.c | 39 ++++++++++++-------- monitor.h | 3 +- net.c | 2 +- qemu-options.hx | 8 +++-- qemu-tool.c | 5 +++ 8 files changed, 131 insertions(+), 36 deletions(-) diff --git a/block.c b/block.c index 785c88e..e9e5613 100644 --- a/block.c +++ b/block.c @@ -555,7 +555,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, if (flags & BDRV_O_SNAPSHOT) { BlockDriverState *bs1; int64_t total_size; - int is_protocol = 0; BlockDriver *bdrv_qcow2; QEMUOptionParameter *options; char tmp_filename[PATH_MAX]; @@ -573,19 +572,17 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags, } total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK; - if (bs1->drv && bs1->drv->protocol_name) - is_protocol = 1; - bdrv_delete(bs1); get_tmp_filename(tmp_filename, sizeof(tmp_filename)); /* Real path is meaningless for protocols */ - if (is_protocol) + if (path_has_protocol(filename)) { snprintf(backing_filename, sizeof(backing_filename), "%s", filename); - else if (!realpath(filename, backing_filename)) + } else if (!realpath(filename, backing_filename)) { return -errno; + } bdrv_qcow2 = bdrv_find_format("qcow2"); options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); diff --git a/block/raw-posix.c b/block/raw-posix.c index c5c9944..42ae2f4 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -28,6 +28,7 @@ #include "block_int.h" #include "module.h" #include "block/raw-posix-aio.h" +#include "monitor.h" #ifdef CONFIG_COCOA #include <paths.h> @@ -185,7 +186,8 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, int bdrv_flags, int open_flags) { BDRVRawState *s = bs->opaque; - int fd, ret; + int fd = -1; + int ret; ret = raw_normalize_devicepath(&filename); if (ret != 0) { @@ -207,13 +209,21 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, if (!(bdrv_flags & BDRV_O_CACHE_WB)) s->open_flags |= O_DSYNC; - s->fd = -1; - fd = qemu_open(filename, s->open_flags, 0644); - if (fd < 0) { - ret = -errno; - if (ret == -EROFS) - ret = -EACCES; - return ret; + if (s->fd == -1) { + fd = qemu_open(filename, s->open_flags, 0644); + if (fd < 0) { + ret = -errno; + if (ret == -EROFS) { + ret = -EACCES; + } + return ret; + } + } else { + fd = dup(s->fd); + if (fd < 0) { + ret = -errno; + return ret; + } } s->fd = fd; s->aligned_buf = NULL; @@ -271,6 +281,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; + s->fd = -1; s->type = FTYPE_FILE; return raw_open_common(bs, filename, flags, 0); } @@ -904,6 +915,74 @@ static BlockDriver bdrv_file = { .create_options = raw_create_options, }; +static int raw_open_fd(BlockDriverState *bs, const char *filename, int flags) +{ + BDRVRawState *s = bs->opaque; + const char *fd_str; + int fd; + + /* extract the file descriptor - fail if it's not fd: */ + if (!strstart(filename, "fd:", &fd_str)) { + return -EINVAL; + } + + if (!qemu_isdigit(fd_str[0])) { + /* get fd from monitor, but don't remove from monitor's fd list */ + fd = qemu_get_fd(fd_str, 0); + if (fd == -1) { + return -EBADF; + } + } else { + char *endptr = NULL; + + fd = strtol(fd_str, &endptr, 10); + if (*endptr || (fd == 0 && fd_str == endptr)) { + return -EBADF; + } + } + + s->fd = fd; + s->type = FTYPE_FILE; + + return raw_open_common(bs, filename, flags, 0); +} + +static void raw_close_fd(BlockDriverState *bs) +{ + const char *fd_str; + + /* if file descriptor is an fdname, remove it from monitor's fd list */ + if (strstart(bs->filename, "fd:", &fd_str)) { + if (!qemu_isdigit(fd_str[0])) { + qemu_get_fd(fd_str, 1); + } + } + + raw_close(bs); +} + +static BlockDriver bdrv_file_fd = { + .format_name = "file", + .protocol_name = "fd", + .instance_size = sizeof(BDRVRawState), + .bdrv_probe = NULL, /* no probe for protocols */ + .bdrv_file_open = raw_open_fd, + .bdrv_read = raw_read, + .bdrv_write = raw_write, + .bdrv_close = raw_close_fd, + .bdrv_flush = raw_flush, + .bdrv_discard = raw_discard, + + .bdrv_aio_readv = raw_aio_readv, + .bdrv_aio_writev = raw_aio_writev, + .bdrv_aio_flush = raw_aio_flush, + + .bdrv_truncate = raw_truncate, + .bdrv_getlength = raw_getlength, + + .create_options = raw_create_options, +}; + /***********************************************/ /* host device */ @@ -1012,6 +1091,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) } #endif + s->fd = -1; s->type = FTYPE_FILE; #if defined(__linux__) { @@ -1184,6 +1264,7 @@ static int floppy_open(BlockDriverState *bs, const char *filename, int flags) BDRVRawState *s = bs->opaque; int ret; + s->fd = -1; s->type = FTYPE_FD; /* open will not fail even if no floppy is inserted, so add O_NONBLOCK */ @@ -1302,6 +1383,7 @@ static int cdrom_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; + s->fd = -1; s->type = FTYPE_CD; /* open will not fail even if no CD is inserted, so add O_NONBLOCK */ @@ -1527,6 +1609,7 @@ static void bdrv_file_init(void) * Register all the drivers. Note that order is important, the driver * registered last will get probed first. */ + bdrv_register(&bdrv_file_fd); bdrv_register(&bdrv_file); bdrv_register(&bdrv_host_device); #ifdef __linux__ diff --git a/migration-fd.c b/migration-fd.c index aee690a..b1ba625 100644 --- a/migration-fd.c +++ b/migration-fd.c @@ -61,7 +61,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon, s = g_malloc0(sizeof(*s)); - s->fd = monitor_get_fd(mon, fdname); + s->fd = monitor_get_fd(mon, fdname, 1); if (s->fd == -1) { DPRINTF("fd_migration: invalid file descriptor identifier\n"); goto err_after_alloc; diff --git a/monitor.c b/monitor.c index 39791dc..d03e762 100644 --- a/monitor.c +++ b/monitor.c @@ -1198,21 +1198,21 @@ static int add_graphics_client(Monitor *mon, const QDict *qdict, QObject **ret_d qerror_report(QERR_DEVICE_NOT_ACTIVE, "spice"); return -1; } - qerror_report(QERR_ADD_CLIENT_FAILED); - return -1; + qerror_report(QERR_ADD_CLIENT_FAILED); + return -1; #ifdef CONFIG_VNC } else if (strcmp(protocol, "vnc") == 0) { - int fd = monitor_get_fd(mon, fdname); - vnc_display_add_client(NULL, fd, skipauth); - return 0; + int fd = monitor_get_fd(mon, fdname, 1); + vnc_display_add_client(NULL, fd, skipauth); + return 0; #endif } else if ((s = qemu_chr_find(protocol)) != NULL) { - int fd = monitor_get_fd(mon, fdname); - if (qemu_chr_add_client(s, fd) < 0) { - qerror_report(QERR_ADD_CLIENT_FAILED); - return -1; - } - return 0; + int fd = monitor_get_fd(mon, fdname, 1); + if (qemu_chr_add_client(s, fd) < 0) { + qerror_report(QERR_ADD_CLIENT_FAILED); + return -1; + } + return 0; } qerror_report(QERR_INVALID_PARAMETER, "protocol"); @@ -2833,7 +2833,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) } } -int monitor_get_fd(Monitor *mon, const char *fdname) +int monitor_get_fd(Monitor *mon, const char *fdname, unsigned char remove) { mon_fd_t *monfd; @@ -2846,10 +2846,12 @@ int monitor_get_fd(Monitor *mon, const char *fdname) fd = monfd->fd; - /* caller takes ownership of fd */ - QLIST_REMOVE(monfd, next); - g_free(monfd->name); - g_free(monfd); + if (remove) { + /* caller takes ownership of fd */ + QLIST_REMOVE(monfd, next); + g_free(monfd->name); + g_free(monfd); + } return fd; } @@ -2857,6 +2859,11 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +int qemu_get_fd(const char *fdname, unsigned char remove) +{ + return cur_mon ? monitor_get_fd(cur_mon, fdname, remove) : -1; +} + static const mon_cmd_t mon_cmds[] = { #include "hmp-commands.h" { NULL, NULL, }, diff --git a/monitor.h b/monitor.h index 4f2d328..f18de19 100644 --- a/monitor.h +++ b/monitor.h @@ -50,7 +50,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs, BlockDriverCompletionFunc *completion_cb, void *opaque); -int monitor_get_fd(Monitor *mon, const char *fdname); +int monitor_get_fd(Monitor *mon, const char *fdname, unsigned char remove); +int qemu_get_fd(const char *fdname, unsigned char remove); void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); diff --git a/net.c b/net.c index d05930c..3482598 100644 --- a/net.c +++ b/net.c @@ -727,7 +727,7 @@ int net_handle_fd_param(Monitor *mon, const char *param) if (!qemu_isdigit(param[0]) && mon) { - fd = monitor_get_fd(mon, param); + fd = monitor_get_fd(mon, param, 1); if (fd == -1) { error_report("No file descriptor named %s found", param); return -1; diff --git a/qemu-options.hx b/qemu-options.hx index d86815d..869320b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -131,7 +131,7 @@ using @file{/dev/cdrom} as filename (@pxref{host_drives}). ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, - "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" + "-drive [file=[fd:]file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" " [,cache=writethrough|writeback|none|unsafe][,format=f]\n" " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n" @@ -144,10 +144,12 @@ STEXI Define a new drive. Valid options are: @table @option -@item file=@var{file} +@item file=[fd:]@var{file} This option defines which disk image (@pxref{disk_images}) to use with this drive. If the filename contains comma, you must double it -(for instance, "file=my,,file" to use file "my,file"). +(for instance, "file=my,,file" to use file "my,file"). @option{fd:}@var{file} +specifies the file descriptor of an already open disk image. +@option{format=}@var{format} is required by @option{fd:}@var{file}. @item if=@var{interface} This option defines on which type on interface the drive is connected. Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. diff --git a/qemu-tool.c b/qemu-tool.c index eb89fe0..a2a0129 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -96,3 +96,8 @@ int64_t qemu_get_clock_ns(QEMUClock *clock) { return 0; } + +int qemu_get_fd(const char *fdname, unsigned char remove) +{ + return -1; +} -- 1.7.3.4

7 19

[libvirt] [PATCH] virsh: Return false if connection or domain is not available
by Osier Yang 23 Aug '11

23 Aug '11

Instead of goto cleanup lable, it will work fine if go to cleanup lable, but it's somewhat waste. And some functions don't check if the "domain == NULL" before trying to free it with virDomainFree(dom), as a result, there is additional error in the log says: error: invalid domain pointer in virDomainFree Which is useless. --- tools/virsh.c | 60 ++++++++++++++++++++++++++++---------------------------- 1 files changed, 30 insertions(+), 30 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 7d849ec..ffb4ced 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5201,10 +5201,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, int ret = -1; if (!vshConnectionUsability(ctl, ctl->conn)) - goto out; + return -1; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) - goto out; + return -1; if (vshCommandOptString(cmd, "path", &path) < 0) goto out; @@ -10519,10 +10519,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) char *xml; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; @@ -10633,10 +10633,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; @@ -10922,10 +10922,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) char *xml; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "source", &source) <= 0) goto cleanup; @@ -11105,10 +11105,10 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; + return false; if (vshCommandOptString(cmd, "target", &target) <= 0) goto cleanup; @@ -11655,11 +11655,11 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain (ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; /* Get the XML configuration of the domain. */ doc = virDomainGetXMLDesc (dom, flags); @@ -11806,11 +11806,11 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) char *name = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "xmlfile", &from) <= 0) buffer = vshStrdup(ctl, "<domainsnapshot/>"); @@ -11902,11 +11902,11 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "name", &name) < 0 || vshCommandOptString(cmd, "description", &desc) < 0) { @@ -11995,11 +11995,11 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; current = virDomainHasCurrentSnapshot(dom, 0); if (current < 0) @@ -12079,11 +12079,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) struct tm time_info; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; numsnaps = virDomainSnapshotNum(dom, 0); @@ -12186,11 +12186,11 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) char *xml = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12245,11 +12245,11 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) xmlXPathContextPtr ctxt = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12311,11 +12311,11 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) virDomainSnapshotPtr snapshot = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12364,11 +12364,11 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; @@ -12427,11 +12427,11 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) bool pad = false; if (!vshConnectionUsability(ctl, ctl->conn)) - goto cleanup; + return false; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) - goto cleanup; + return false; while ((opt = vshCommandOptArgv(cmd, opt))) { if (pad) -- 1.7.6

2 5

[libvirt] ignore vs. error when inappropriate or unrecognized attributes/elements are present in XML
by Laine Stump 23 Aug '11

23 Aug '11

This is related to: https://bugzilla.redhat.com/show_bug.cgi?id=638633#c14 I had started to reply to it in the comments of the bug, but my reply became too long, and expanded into an issue wider than that single bug, so I figured I'd better discuss it here instead. A synopsis: It's possible to specify a script to be executed when bringing up a guest interface device. This is done by adding a <script path='xxx'/> element to the <interface> definition. The bug report was that <script path='blah'/> was added to an <interface type='bridge'> definition in the config for a qemu domain, and nobody complained that 'blah' didn't exist. This eventually led to the realization that when interface type='bridge' and driver=qemu, <script> isn't used, so it's ignored. More exactly, the script element is only recognized by the parser when interface type == 'bridge' or 'ethernet', and is only used by the hypervisor driver when: 1) the interface type='bridge' and the xen driver is being used. or 2) the interface type='ethernet' and the qemu driver is being used. After this all came to light, in comment 14, Dave says: > That's being the case, we need to explicitly reject the attempt to specify a > script. (implied: when interface type='bridge' and the hypervisor is qemu). On the surface that sounds like a good idea, but it's opening a pretty big can of worms. Specifically for this issue, this rejection must take place in the hypervisor because different hypervisors support scripts for different types of interfaces - a script for type='bridge' is perfectly acceptable for xen, but not for qemu. That can be handled with an error log in the qemu driver in the case of type='bridge'. But if someone defines <interface type='network'> and specifies a script, the parser ignores the <script> element (because normally it's stored either in a union that's only valid when type='bridge' or a different union only valid when type='ethernet'), so qemu has no way of knowing a script was specified and therefore can't log an error. That's not a terribly difficult problem, though. Just a bit more code than a simple "else { log an error; }" somewhere in the code - the parser needs to be changed to move the script attribute out of the unions and into the main struct, and always populate it, then the drivers need to be taught to log an error when appropriate). But if we're doing that for the <script> subelement of <interface>, we're creating a prerequisite for what's expected in the case of all the *other* attributes/elements that are currently ignored under similar circumstances by libvirt's parsers. Essentially anything that's in a union contained in any of the virXxxDef structs is probably treated the same way. As a simple first example, look at "keymap", which is an attribute of a domain's devices/graphics element, but only valid if the type is spice or vnc. The parser ignores it in all other cases, and since it's stored in data.spice or data.vnc, there's not even a possibility of qemu (or other hypervisor) indicating any kind of error when someone specifies e.g. <graphics type='sdl' 'keymap='blah' .../>. Similarly, if someone were to add "tlsPort='5910' to a <graphics> of any type other than spice. Of course, from the point of view of all the code associated with <graphics type='vnc' ... />, this is no different than if someone had added "frozzle='fib'" - it's totally unexpected (in this case by any type), so it's ignored. Actually, I can see now there are several different classes of this problem. Here are the first few that come to mind: 1) an attribute/element is completely unknown/unexpected in all cases (e.g. "frozzle='fib'" anywhere, or more insidious, something that *looks* correct, but isn't, e.g. "<script name='/path/to/script'/>"*) 2) an attribute/element is useful/expected only when some other attribute is set to a particular value (usually one called "type", but could be something else), for example keymap='blah' is only expected in a <graphics> element when type='spice' or type='vnc'. 3) an attribute/element is useful/expected only for certain combinations of the value of some other attribute and which driver is using the element, e.g. the subject of this bug - script='blah' is only expected when type='bridge' and it's used by the Xen driver, or type='ethernet' and it's used by the qemu driver. So what are the rules of engagement for these various cases? When do we ignore, when do we log an error during parsing, and when do we log an error in the code that's using the parsed data? (*Another example of (1) from recent real-life - during testing of https://bugzilla.redhat.com/show_bug.cgi?id=727982, a QA person claimed that the bugt hadn't been fixed because the hosts file wasn't being updated; it turned out that they were adding the new <host> element at the toplevel under <network> rather than inside the <dhcp> subelement, so the parser was ignoring it (since it's unrecognized at that level) rather than logging an error).

3 7

[libvirt] Draft document about source control for type of pools
by Lei Li 23 Aug '11

23 Aug '11

Hi Daniel, The last version of my patch fixed bug #611823 prohibit pools with duplicate storage, you gave some comments that should do check by source info instead of target path I used. These days I view the code and existing documents about storage pool, and tried to code based on your comments, but looks like it is turning out to be more complex. First, I think it is possible to make clear what it means for a pool to be a duplicate. Below is the detailed source info be used for each type of pool according to the existing documents and the main item to control. For details of each type of pool: VIR_STORAGE_POOL_DIR, /* For dir type, use source->dir only(But in the code and example XML file,it use target path).*/ A pool with a type of dir provides the means to manage files within a directory. VIR_STORAGE_POOL_FS, /* For fs type, use source->device->path.*/ This is a variant of the directory pool. Instead of creating a directory on an existing mounted filesystem though, it expects a source block device to be named. This block device will be mounted and files managed in the directory of its mount point. VIR_STORAGE_POOL_NETFS, /* For netfs, use host name and source->dir */ This is a variant of the filesystem pool. Instead of requiring a local block device as the source, it requires the name of a host and path of an exported directory. It will mount this network filesystem and manage files within the directory of its mount point. VIR_STORAGE_POOL_LOGICAL, /* For logical, use source->device->path. */ This provides a pool based on an LVM volume group. For a pre-defined LVM volume group, simply providing the group name is sufficient,while to build a new group requires providing a list of source devices to serve as physical volumes. VIR_STORAGE_POOL_DISK, /* For disk type, use source->device->path.*/ This provides a pool based on a physical disk. Volumes are created by adding partitions to the disk. VIR_STORAGE_POOL_ISCSI, /* For ISCSI type, use source->device->path. */ This provides a pool based on an iSCSI target. VIR_STORAGE_POOL_SCSI, /* For SCSI type, use source->adapter name. */ This provides a pool based on a SCSI HBA. VIR_STORAGE_POOL_MPATH, This provides a pool that contains all the multipath devices on the host. Volume creating is not supported via the libvirt APIs, so we will just ignore this type. After a preliminary investigation, for source element, source->device->path, -Pool type: fs, logical, disk, ISCSI. -May only occur once. source->dir, -Pool type: dir, netfs. -May only occur once. source->adapter -Pool type: SCSI. -May only occur once. are the main required location info, and we can control the duplicate storage based on these three item above for related type of pool. Second, I have tried to do check by source info for a test, when I debug, I found that pools->objs[]->def->source.* which come from virConnectPtr where all existing pools info be keeped == NULL, but the current target pool virStoragePoolDefPtr def can get source field value, so I think libvirt didn't get source field info in the pools list where we do check at all today. Do you think we should do check based on this? Or any comments for question one and two. After you confirmed then I will work on code. Thank you. -- Lei

2 1

[libvirt] hi, Is libvirt plan to support open vswitch in the future? thanks
by c02925 23 Aug '11

23 Aug '11

Hello, Is libvirt plan to support open vswitch in the future, or there is already has a blueprint about it. Has anyone else thought about this problem? Any related information is welcome. Many thanks, zhifeng cai. <http://www.linkedin.com/in/aliyev>

1 0

[libvirt] [PATCH] qemu: Substitute VIR_ERR_NO_SUPPORT with VIR_ERR_OPERATION_INVALID
by Osier Yang 23 Aug '11

23 Aug '11

* src/qemu/qemu_monitor_text.c: Error like "this function is not supported by the connection driver" is confused obviously. --- src/qemu/qemu_monitor_text.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index d296675..4a7c242 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -713,7 +713,7 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, * to detect if qemu supports the command. */ if (strstr(info, "\ninfo ")) { - qemuReportError(VIR_ERR_NO_SUPPORT, + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("'info blockstats' not supported by this qemu")); goto cleanup; @@ -1388,7 +1388,7 @@ int qemuMonitorTextMigrate(qemuMonitorPtr mon, /* If the command isn't supported then qemu prints: * unknown command: migrate" */ if (strstr(info, "unknown command:")) { - qemuReportError(VIR_ERR_NO_SUPPORT, + qemuReportError(VIR_ERR_OPERATION_INVALID, _("migration to '%s' not supported by this qemu: %s"), dest, info); goto cleanup; } @@ -1652,7 +1652,7 @@ int qemuMonitorTextAddPCIHostDevice(qemuMonitorPtr mon, } if (strstr(reply, "invalid type: host")) { - qemuReportError(VIR_ERR_NO_SUPPORT, "%s", + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("PCI device assignment is not supported by this version of qemu")); goto cleanup; } -- 1.7.6

2 4

[libvirt] [PATCH] virsh: Print error if specified bandwidth is invalid for blockjob
by Osier Yang 23 Aug '11

23 Aug '11

It's strange that the command fails but without any error if one specifies as not a number. --- tools/virsh.c | 21 ++++++++++++++++----- 1 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ffb4ced..97512b5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5209,17 +5209,28 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(cmd, "path", &path) < 0) goto out; - if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) + if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); goto out; + } - if (mode == VSH_CMD_BLOCK_JOB_ABORT) + if (mode == VSH_CMD_BLOCK_JOB_ABORT) { + vshPrint(ctl, "abort"); ret = virDomainBlockJobAbort(dom, path, 0); - else if (mode == VSH_CMD_BLOCK_JOB_INFO) + } + else if (mode == VSH_CMD_BLOCK_JOB_INFO) { + vshPrint(ctl, "info\n"); ret = virDomainGetBlockJobInfo(dom, path, info, 0); - else if (mode == VSH_CMD_BLOCK_JOB_SPEED) + vshPrint(ctl, "ret = %d\n", ret); + } + else if (mode == VSH_CMD_BLOCK_JOB_SPEED) { + vshPrint(ctl, "speed"); ret = virDomainBlockJobSetSpeed(dom, path, bandwidth, 0); - else if (mode == VSH_CMD_BLOCK_JOB_PULL) + } + else if (mode == VSH_CMD_BLOCK_JOB_PULL) { + vshPrint(ctl, "pull"); ret = virDomainBlockPull(dom, path, bandwidth, 0); + } out: virDomainFree(dom); -- 1.7.6

2 3

[libvirt] Adding migration speed to domainXML
by Jim Fehlig 23 Aug '11

23 Aug '11

I finally have some time to investigate Eric's suggestion [1] to add virDomainMigrateGetMaxSpeed API and track current value in domainXML. WRT the XML representation, it seems this best fits under the existing lifecycle controls, e.g. a new "on_migrate" element. I'm not sure if this should be kept simple and rather flat such as <on_migrate max-speed='1000000000' /> or a little more expressive and extensible with something like <on_migrate> <bandwidth peak='1000000000' /> </on_migrate> Any comments on these representations or perhaps alternative suggestions? Regards, Jim [1] https://www.redhat.com/archives/libvir-list/2011-August/msg00224.html

1 0

[libvirt] Fwd: [virt-tools-list] virt-manager, dependency question
by Jason Helfman 22 Aug '11

22 Aug '11

Can anyone please confirm this for me? Thanks! Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5

3 2

[libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob
by Osier Yang 22 Aug '11

22 Aug '11

* src/qemu/qemu_monitor_json.c: Handle error "CommandNotFound" and report the error. * src/qemu/qemu_monitor_text.c: If a sub info command is not found, it prints the output of "help info", for other commands, "unknown command" is printed. Without this patch, libvirt always report: An error occurred, but the cause is unknown --- src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++----------- src/qemu/qemu_monitor_text.c | 47 +++++++++++++++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..715b26e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2909,20 +2909,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; - - if (mode == BLOCK_JOB_ABORT) - cmd = qemuMonitorJSONMakeCommand("block_job_cancel", - "s:device", device, NULL); - else if (mode == BLOCK_JOB_INFO) - cmd = qemuMonitorJSONMakeCommand("query-block-jobs", NULL); - else if (mode == BLOCK_JOB_SPEED) - cmd = qemuMonitorJSONMakeCommand("block_job_set_speed", - "s:device", device, - "U:value", bandwidth * 1024ULL * 1024ULL, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "query-block-jobs"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, "U:value", + bandwidth * 1024ULL * 1024ULL, NULL); - else if (mode == BLOCK_JOB_PULL) - cmd = qemuMonitorJSONMakeCommand("block_stream", - "s:device", device, NULL); + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", + device, NULL); + } if (!cmd) return -1; @@ -2939,6 +2944,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..d296675 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3031,18 +3031,24 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, char *cmd = NULL; char *reply = NULL; int ret; - - if (mode == BLOCK_JOB_ABORT) - ret = virAsprintf(&cmd, "block_job_cancel %s", device); - else if (mode == BLOCK_JOB_INFO) - ret = virAsprintf(&cmd, "info block-jobs"); - else if (mode == BLOCK_JOB_SPEED) - ret = virAsprintf(&cmd, "block_job_set_speed %s %llu", device, + const char *cmd_name = NULL; + + if (mode == BLOCK_JOB_ABORT) { + cmd_name = "block_job_cancel"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else if (mode == BLOCK_JOB_INFO) { + cmd_name = "info block-jobs"; + ret = virAsprintf(&cmd, "%s", cmd_name); + } else if (mode == BLOCK_JOB_SPEED) { + cmd_name = "block_job_set_speed"; + ret = virAsprintf(&cmd, "%s %s %llu", cmd_name, device, bandwidth * 1024ULL * 1024ULL); - else if (mode == BLOCK_JOB_PULL) - ret = virAsprintf(&cmd, "block_stream %s", device); - else + } else if (mode == BLOCK_JOB_PULL) { + cmd_name = "block_stream"; + ret = virAsprintf(&cmd, "%s %s", cmd_name, device); + } else { return -1; + } if (ret < 0) { virReportOOMError(); @@ -3056,6 +3062,27 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + /* Check error: Command not found, for "info block-jobs" command. + * If qemu monitor command "info" doesn't support one sub-command, + * it will print the output of "help info" instead, so simply check + * if "info version" is in the output. + */ + if (mode == BLOCK_JOB_INFO) { + if (strstr(reply, "info version")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + } else { + if (strstr(reply, "unknown command")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Command '%s' is not found"), cmd_name); + ret = -1; + goto cleanup; + } + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.6

3 3

[libvirt] [PATCH] blockjob: Report correct error when missing qemu support
by Adam Litke 22 Aug '11

22 Aug '11

When executing blockjob monitor commands, we are not checking for CommandNotFound errors. This results in 'unexpected error' messages when using a qemu without support for the required commands. Fix this up by checking for CommandNotFound errors for QMP and detecting this condition for the text monitor. This fixes the problem reported in Bug 727502. Signed-off-by: Adam Litke <agl(a)us.ibm.com> --- src/qemu/qemu_monitor_json.c | 3 +++ src/qemu/qemu_monitor_text.c | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..4c8a08b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2939,6 +2939,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, else if (qemuMonitorJSONHasError(reply, "NotSupported")) qemuReportError(VIR_ERR_OPERATION_INVALID, _("Operation is not supported for device: %s"), device); + else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Operation is not supported")); else qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..d9cad2d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -271,6 +271,19 @@ qemuMonitorTextCommandWithFd(qemuMonitorPtr mon, scm_fd, reply); } +/* Check monitor output for evidence that the command was not recognized. + * For 'info' commands, qemu returns help text. For other commands, qemu + * returns 'unknown command:'. + */ +static int +qemuMonitorTextCommandNotFound(const char *reply) +{ + if (strstr(reply, "unknown command:")) + return 1; + if (strstr(reply, "info version -- show the version of QEMU")) + return 1; + return 0; +} static int qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, @@ -3056,6 +3069,13 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, goto cleanup; } + if (qemuMonitorTextCommandNotFound(reply)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Operation is not supported")); + ret = -1; + goto cleanup; + } + ret = qemuMonitorTextParseBlockJob(reply, device, info); cleanup: -- 1.7.3

2 1

[libvirt] [RFC 0/4] Report disk latency information
by Osier Yang 22 Aug '11

22 Aug '11

The QEMU side patches are not posted yet, but considering what these patche series need is only a fixed interface provided by QEMU, posting it for early reviewing, won't push it before QEMU patches are got pushed even it got ACK. Osier

6 13

[libvirt] [PATCH] remote: Switch to TLS when connecting to remote socket
by Michal Privoznik 22 Aug '11

22 Aug '11

If users wants to connect to remote unix socket, e.g. 'qemu+unix://<remote>/system' currently the <remote> part is ignored, ending up connecting to localhost. This patch solves this issue by creating TLS connection. --- src/remote/remote_driver.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e5bfa4b..630c8ea 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -325,9 +325,14 @@ doRemoteOpen (virConnectPtr conn, } else { if (STRCASEEQ (transport_str, "tls")) transport = trans_tls; - else if (STRCASEEQ (transport_str, "unix")) - transport = trans_unix; - else if (STRCASEEQ (transport_str, "ssh")) + else if (STRCASEEQ (transport_str, "unix")) { + /* When connecting to unix socket on remote + * machine use TLS to connect to it */ + if (conn->uri->server) + transport = trans_tls; + else + transport = trans_unix; + } else if (STRCASEEQ (transport_str, "ssh")) transport = trans_ssh; else if (STRCASEEQ (transport_str, "ext")) transport = trans_ext; -- 1.7.3.4

2 1

[libvirt] [PATCH] [BUGFIX][PATCH] send-key: fix scan keycode map
by KAMEZAWA Hiroyuki 22 Aug '11

22 Aug '11

Now, bad key-code in send-key can cause segmentation fault in libvirt. (example) % virsh send-key --codeset win32 12 error: End of file while reading data: Input/output error This is caused by overrun at scanning keycode array. Fix it. Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com> --- src/util/virkeycode.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c index 4d54060..0d42767 100644 --- a/src/util/virkeycode.c +++ b/src/util/virkeycode.c @@ -118,7 +118,7 @@ static int __virKeycodeValueTranslate(unsigned int from_offset, { int i; - for (i = 0; ARRAY_CARDINALITY(virKeycodes); i++) { + for (i = 0; i < ARRAY_CARDINALITY(virKeycodes); i++) { if (getfield(virKeycodes + i, unsigned short, from_offset) == key_value) return getfield(virKeycodes + i, unsigned short, to_offset); } -- 1.7.4.1

2 1

[libvirt] portability issue on Centos 6.0
by Daniel Veillard 22 Aug '11

22 Aug '11

I just tred compiling 0.9.4 and git head on Centos 6.0, for the possible libvirt.org server replacement and it fails in both cases with: CC libvirtd-remote.o remote.c: In function 'remoteDispatchAuthPolkit': remote.c:2161: error: invalid initializer This correspond to the following macro PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s", virNetServerClientGetFD(client), REMOTE_AUTH_POLKIT, ident); I'm a bit lost how the expansion could lead to this and why, unless the DTrace support in Centos 6 is slightly different than on RHEL, I will look but if someone has an idea :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] How to avoid the disk still persistent after no persistent hotplug and with restarting libvirtd then.
by Osier Yang 22 Aug '11

22 Aug '11

Libvirt loads the domain conf from the status file (/var/run/libvirt/qemu/$dom.xml) if the domain is running, the problem if one restart libvirtd just after some changing on the domain conf, and then destroy/start the guest. libvirt will never known what the original domain conf is then. (vm->newdef is NULL at this moment, generally libvirt restore the domain conf to the original conf (vm->newDef) when destroying and shutdown). Following is a demo of the problem: 1) hotplug a disk to a running domain without "--persistent" specified. 2) restart libvirtd 3) Destroy/start the guest 4) you will see the hotpluged disk is still there. The solution in my mind is to add some attribute to the status XML, such as "live_updated=1", and do the checking when loading the domain conf from status XML: * if "live_updated=1", then loads the domain conf from the persistent domain XML. But the problem is: For a persistent domain, if one made changes on the domain conf both persistently and transiently, we have no way to known which part are changed. Loading from the persistent domain conf is not correct either. For a transient domain, we still have no way to get the original domain conf, that means one will still be surprised when seeing the the disk is still there. Any idea on this? Thanks Osier

2 3

[libvirt] Proposal: A place to store metadata about guests
by Richard W.M. Jones 22 Aug '11

22 Aug '11

I think we need a place to store general metadata about guests. Here are some uses: - Store data about available installer ISOs, guests which are archived and not known to libvirt. - Replace the guest inspection thread in virt-manager. virt-manager would just collect the data from the store. The inspection data would be generated by another process. - Expand on <description/> field in the libvirt XML, and satisfy various requests we've had for a place to store metadata and extend libvirt XML. - Store data on behalf of applications, esp. virt-manager. I'm not envisaging a database or daemon for this. Just a simple directory per host containing flat files. Perhaps we can add some tools or a library to make creation/querying of these files easy. It would be nice if the database was made available over the network (Avahi?). You could also use this database to answer questions like: - "Do I have a Debian wheezy 32 bit guest anywhere on my network?" - "Does any guest have libpng <= <vulnerable-version> installed?" - "Bring up a Fedora 14 instance" (the tools would have to locate a suitable install ISO and put together the correct virt-install command line) There is some crossover with VDSM and even RHEV-M here. Thoughts? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

3 8

[libvirt] [libvirt-snmp][PATCH 1/3] subagent: Avoid dead code
by Michal Privoznik 22 Aug '11

22 Aug '11

Detected by Coverity --- src/libvirtMib_subagent.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/libvirtMib_subagent.c b/src/libvirtMib_subagent.c index f5749be..ee99fb9 100644 --- a/src/libvirtMib_subagent.c +++ b/src/libvirtMib_subagent.c @@ -60,7 +60,7 @@ main (int argc, char **argv) { /* Defs for arg-handling code: handles setting of policy-related variables */ int ch; extern char *optarg; - int dont_fork = 0, use_syslog = 0; + int dont_fork = 0, use_syslog = 1; char *agentx_socket = NULL; while ((ch = getopt(argc, argv, "D:fHLMx:")) != EOF) -- 1.7.3.4

1 2

[libvirt] [PATCH v2] qemu: Allow graceful domain destroy
by Michal Privoznik 22 Aug '11

22 Aug '11

This patch introduces support for domain destroying via 'quit' monitor command which gives qemu time to flush caches and therefore prevent disks corruption. However, qemu can be unresponsive and to prevent waiting indefinitely, execute command in a separate thread and wait reasonable time (QEMU_QUIT_WAIT_SECONDS) on a condition. If we hit timeout, qemu is qemuProcessKill'ed which causes monitor close and therefore also thread being terminable. The synchronization between qemu driver and monitor-quit thread is done through mutex and condition. However, this alone is not enough. If a condition is signalized but without anybody listening signal is lost. To prevent this a boolean variable is used that is set iff nobody is listening but condition would be signalized, or iff driver is waiting on given condition. --- diff to v1: - more debug printings - don't test (!flags) but (!(flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE)) include/libvirt/libvirt.h.in | 11 ++- src/qemu/qemu_driver.c | 146 ++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 2 + src/qemu/qemu_monitor_json.c | 22 ++++++ src/qemu/qemu_monitor_json.h | 1 + src/qemu/qemu_monitor_text.c | 20 ++++++ src/qemu/qemu_monitor_text.h | 1 + tools/virsh.c | 9 ++- 9 files changed, 213 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aa29fb6..0610b53 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,13 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ + +typedef enum { + VIR_DOMAIN_DESTROY_FLUSH_CACHE = 1 << 0, /* In hypervisors supporting this, + try to get cache flushed prior + to destroying the domain */ +} virDomainDestroyFlagsValues; + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c8dda73..44f4bbd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1564,6 +1564,47 @@ cleanup: return ret; } +struct qemuDomainDestroyHelperData { + struct qemud_driver *driver; + virDomainObjPtr vm; + qemuDomainObjPrivatePtr priv; + virMutex lock; + virCond cond; + bool guard; +}; + +static void +qemuDomainDestroyHelper(void *opaque) +{ + struct qemuDomainDestroyHelperData *data = opaque; + struct qemud_driver *driver = data->driver; + virDomainObjPtr vm = data->vm; + qemuDomainObjPrivatePtr priv = data->priv; + + /* Job was already obtained by caller */ + VIR_DEBUG("Destroy thread started, sending 'quit' command."); + qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuMonitorQuit(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + VIR_DEBUG("Command finished."); + + /* To avoid loosing signal, we need to remember + * we tried to send one, but nobody was waiting + * for it. + */ + virMutexLock(&data->lock); + if (data->guard) { + VIR_DEBUG("Driver is listening, signalize on condition"); + virCondSignal(&data->cond); + } else { + VIR_DEBUG("Nobody's listening, setting guard variable"); + data->guard = true; + } + virMutexUnlock(&data->lock); + VIR_DEBUG("Destroy thread exiting"); +} + +#define QEMU_QUIT_WAIT_SECONDS 5 static int qemuDomainDestroyFlags(virDomainPtr dom, @@ -1574,8 +1615,13 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + bool use_thread = false; + bool use_kill = false; + virThread destroy_thread; + struct qemuDomainDestroyHelperData data; + unsigned long long then; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_FLUSH_CACHE, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1590,12 +1636,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, priv = vm->privateData; priv->fakeReboot = false; - /* Although qemuProcessStop does this already, there may - * be an outstanding job active. We want to make sure we - * can kill the process even if a job is active. Killing - * it now means the job will be released - */ - qemuProcessKill(vm); + if (!(flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE)) { + /* Although qemuProcessStop does this already, there may + * be an outstanding job active. We want to make sure we + * can kill the process even if a job is active. Killing + * it now means the job will be released + */ + qemuProcessKill(vm); + } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; @@ -1606,6 +1654,78 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto endjob; } + if (flags & VIR_DOMAIN_DESTROY_FLUSH_CACHE) { + /* Try to shutdown domain gracefully. + * Send 'quit' to qemu. However, qemu can be unresponsive. + * Therefore create a separate thread in which we execute + * that monitor comand. Wait max QEMU_QUIT_WAIT_SECONDS. + */ + VIR_DEBUG("Trying to gracefully shutdown %s", vm->def->name); + if (virMutexInit(&data.lock) < 0) { + virReportOOMError(); + goto endjob; + } + + if (virCondInit(&data.cond) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize thread condition")); + virMutexDestroy(&data.lock); + goto endjob; + } + + data.driver = driver; + data.vm = vm; + data.priv = priv; + data.guard = false; + + VIR_DEBUG("Spawning thread to run 'quit' command"); + if (virThreadCreate(&destroy_thread, true, + qemuDomainDestroyHelper, &data) < 0) { + virReportSystemError(errno, "%s", + _("unable to create destroy thread")); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + goto endjob; + } + use_thread = true; + + if (virTimeMs(&then) < 0) + goto endjob; + + then += QEMU_QUIT_WAIT_SECONDS * 1000; + + /* How synchronization with destroy thread works: + * Basically wait on data.cond. But because conditions + * does not 'remember' that they have been signalized + * data.guard is used. Practically, data.guard says + * to destroy thread if we are waiting on condition + * and to us whether we should even try. + */ + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + VIR_DEBUG("Waiting for thread to exit"); + if (virCondWaitUntil(&data.cond, &data.lock, then) < 0) { + if (errno == ETIMEDOUT) { + VIR_DEBUG("Condition timeout"); + use_kill = true; + data.guard = false; + } else { + virReportSystemError(errno, "%s", _("unable to wait " + "on condition")); + virMutexUnlock(&data.lock); + goto endjob; + } + } + } else { + VIR_DEBUG("Guard variable set, thread exited"); + } + virMutexUnlock(&data.lock); + + if (use_kill) + qemuProcessKill(vm); + } + qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_DESTROYED); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -1626,6 +1746,18 @@ endjob: vm = NULL; cleanup: + if (use_thread) { + VIR_DEBUG("Thread used, clean it up"); + virMutexLock(&data.lock); + if (!data.guard) { + data.guard = true; + ignore_value(virCondWait(&data.cond, &data.lock)); + } + virMutexUnlock(&data.lock); + ignore_value(virCondDestroy(&data.cond)); + virMutexDestroy(&data.lock); + virThreadJoin(&destroy_thread); + } if (vm) virDomainObjUnlock(vm); if (event) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..41b9c5c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2475,3 +2475,16 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, ret = qemuMonitorTextBlockJob(mon, device, bandwidth, info, mode); return ret; } + +int qemuMonitorQuit(qemuMonitorPtr mon) +{ + int ret; + + VIR_DEBUG("mon=%p", mon); + + if (mon->json) + ret = qemuMonitorJSONQuit(mon); + else + ret = qemuMonitorTextQuit(mon); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..3fe6bb9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -475,6 +475,8 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorQuit(qemuMonitorPtr mon); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7adfb26..1f078cd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2956,3 +2956,25 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int qemuMonitorJSONQuit(qemuMonitorPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("quit", NULL); + + if (!cmd) + return -1; + + ret =qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..2a7df76 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -231,4 +231,5 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorJSONQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 52d924a..ca9c21b 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -3063,3 +3063,23 @@ cleanup: VIR_FREE(reply); return ret; } + + +int qemuMonitorTextQuit(qemuMonitorPtr mon) +{ + const char *cmd = "quit"; + char *reply = NULL; + int ret = -1; + + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..9ade938 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -224,4 +224,5 @@ int qemuMonitorTextBlockJob(qemuMonitorPtr mon, virDomainBlockJobInfoPtr info, int mode); +int qemuMonitorTextQuit(qemuMonitorPtr mon); #endif /* QEMU_MONITOR_TEXT_H */ diff --git a/tools/virsh.c b/tools/virsh.c index 7d849ec..4c7962b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2567,6 +2567,8 @@ static const vshCmdInfo info_destroy[] = { static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"flush", VSH_OT_BOOL, 0, N_("try to flush hypervisor's cache prior to " + "destroying the domain")}, {NULL, 0, 0, NULL} }; @@ -2576,6 +2578,7 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = true; const char *name; + int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2583,7 +2586,11 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainDestroy(dom) == 0) { + if (vshCommandOptBool(cmd, "flush")) + flags |= VIR_DOMAIN_DESTROY_FLUSH_CACHE; + + if ((!flags && virDomainDestroy(dom) == 0) || + virDomainDestroyFlags(dom, flags) == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); } else { vshError(ctl, _("Failed to destroy domain %s"), name); -- 1.7.3.4

1 0