[libvirt] Do not fail migration if the media is ejected inside guest.

The whole mail title should be "Do not fail migration if the media is ejected inside guest and the media source is removed". Following 2 patches are trying to solve it, please see the patch descriptions for more detail. BZ# https://bugzilla.redhat.com/show_bug.cgi?id=725673 [PATCH 1/2] qemu: Get the media status of removable block device [PATCH 2/2] qemu: Clear source path of removable block if it is Osier

Intorduce new monitor functions to get the media status (ejected or inserted) of removable block device via qemu monitor command "info block". QEMU upstream will expose the the media status like: cd: type=cdrom removable=1 locked=0 ejected=0 The related patch: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00408.html Although it's unlikely to expose other information of removable block device in future via the new function, returned the info in argument "*ejected" just in case of there is new requirement to expose other info. --- src/qemu/qemu_monitor.c | 20 ++++++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 14 ++++++ src/qemu/qemu_monitor_json.h | 4 +- src/qemu/qemu_monitor_text.c | 105 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + 6 files changed, 148 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..7a41e8f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1227,6 +1227,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; } +int qemuMonitorGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + int ret; + VIR_DEBUG("mon=%p dev=%s", mon, devname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetRemovableBlockMediaStatus(mon, devname, ejected); + else + ret = qemuMonitorTextGetRemovableBlockMediaStatus(mon, devname, ejected); + return ret; +} + int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f241c9e..e533cbd 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -215,6 +215,9 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *wr_req, long long *wr_bytes, long long *errs); +int qemuMonitorGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected); int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, const char *devname, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..f59489d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1519,6 +1519,20 @@ cleanup: return ret; } +int qemuMonitorJSONGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + /* XXX: No QMP command like "info block" which can get the + * block device information yet. + */ + if (qemuMonitorCheckHMP(mon, "info block")) { + return qemuMonitorTextGetRemovableBlockMediaStatus(mon, devname, + ejected); + } else { + return -1; + } +} int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, const char *password) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9512793..33158c9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -70,7 +70,9 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent); - +int qemuMonitorJSONGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected); int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, const char *password); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 7bf733d..02ed82f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -786,6 +786,111 @@ int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, return -1; } +/* Get the media status (ejected or inserted) of removable block + * device. + * + * Return -1 on failure, 0 on success. + */ +int qemuMonitorTextGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + int ret = -1; + char *info = NULL; + char *dummy; + const char *p, *eol; + int devnamelen = strlen(devname); + + if (qemuMonitorHMPCommand (mon, "info block", &info) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("'info block' command failed")); + goto cleanup; + } + + /* If the command isn't supported then qemu prints the supported + * info commands, so the output starts "info ". Since this is + * unlikely to be the name of a block device, we can use this + * to detect if qemu supports the command. + */ + if (strstr(info, "\ninfo ")) { + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", + _("'info block' not supported by this qemu")); + goto cleanup; + } + + /* The output format of "info block" is like: + * disk: type=hd removable=0 file=/var/lib/libvirt/images/test.img ro=0 \ + * drv=raw encrypted=0 + * cd: type=cdrom removable=1 locked=0 ejected=0 (without media inserted) + * (repeated for each block device) + */ + p = info; + + while (*p) { + /* New QEMU has separate names for host & guest side of the disk + * and libvirt gives the host side a 'drive-' prefix. The passed + * in devname is the guest side though + */ + if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) + p += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQLEN (p, devname, devnamelen) + && p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + + eol = strchr (p, '\n'); + if (!eol) + eol = p + strlen (p); + + p += devnamelen + 2; /* Skip to first label. */ + + while (*p) { + VIR_WARN("p = %s", p); + + /* Make sure the device is removable, we don't want to waste + * time on querying a not removable device. + */ + if (!strstr(p, "ejected=")) { + qemuReportError (VIR_ERR_OPERATION_INVALID, + _("device %s is not removable"), devname); + return -1; + } + + if (STRPREFIX (p, "ejected=")) { + p += strlen("ejected="); + + if (virStrToLong_ui (p, &dummy, 10, ejected) == -1) { + qemuReportError (VIR_ERR_INTERNAL_ERROR, + _("error reading ejected: %s"), p); + return -1; + } else { + break; + } + } + + /* Skip to next label. */ + p = strchr (p, ' '); + if (!p || p >= eol) break; + p++; + } + ret = 0; + goto cleanup; + } + + /* Skip to next line. */ + p = strchr (p, '\n'); + if (!p) break; + p++; + } + + /* If we reach here then the device was not found. */ + qemuReportError (VIR_ERR_INVALID_ARG, + _("no stats found for device %s"), devname); + + cleanup: + VIR_FREE(info); + return ret; +} static int qemuMonitorSendVNCPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index b250738..bfd1f7e 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -67,6 +67,9 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon, const char *devname, unsigned long long *extent); +int qemuMonitorTextGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected); int qemuMonitorTextSetVNCPassword(qemuMonitorPtr mon, const char *password); -- 1.7.6

On Fri, Aug 12, 2011 at 09:34:41PM +0800, Osier Yang wrote:
Intorduce new monitor functions to get the media status (ejected or inserted) of removable block device via qemu monitor command "info block".
QEMU upstream will expose the the media status like: cd: type=cdrom removable=1 locked=0 ejected=0
The related patch: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00408.html
Hmm, that patch isn't merged upstream yet. We ought to wait for it to be merged, since historically discussions about this kind of thing on QEMU-devel have been quiet unpredictable.
Although it's unlikely to expose other information of removable block device in future via the new function, returned the info in argument "*ejected" just in case of there is new requirement to expose other info. --- src/qemu/qemu_monitor.c | 20 ++++++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 14 ++++++ src/qemu/qemu_monitor_json.h | 4 +- src/qemu/qemu_monitor_text.c | 105 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + 6 files changed, 148 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..7a41e8f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1227,6 +1227,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; }
+int qemuMonitorGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected)
The last arg should really be 'bool *ejected'
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..f59489d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1519,6 +1519,20 @@ cleanup: return ret; }
+int qemuMonitorJSONGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + /* XXX: No QMP command like "info block" which can get the + * block device information yet. + */
This is not correct, it is simply called 'query-block' Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

于 2011年08月12日 21:11, Daniel P. Berrange 写道:
On Fri, Aug 12, 2011 at 09:34:41PM +0800, Osier Yang wrote:
Intorduce new monitor functions to get the media status (ejected or inserted) of removable block device via qemu monitor command "info block".
QEMU upstream will expose the the media status like: cd: type=cdrom removable=1 locked=0 ejected=0
The related patch: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00408.html Hmm, that patch isn't merged upstream yet. We ought to wait for it to be merged, since historically discussions about this kind of thing on QEMU-devel have been quiet unpredictable.
Yes, this is just for early reviewing, we need to wait qemu patch is pushed even these patches are ok. I don't known it's quite unpredictable yet. :)
Although it's unlikely to expose other information of removable block device in future via the new function, returned the info in argument "*ejected" just in case of there is new requirement to expose other info. --- src/qemu/qemu_monitor.c | 20 ++++++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 14 ++++++ src/qemu/qemu_monitor_json.h | 4 +- src/qemu/qemu_monitor_text.c | 105 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + 6 files changed, 148 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..7a41e8f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1227,6 +1227,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; }
+int qemuMonitorGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) The last arg should really be 'bool *ejected'
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..f59489d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1519,6 +1519,20 @@ cleanup: return ret; }
+int qemuMonitorJSONGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + /* XXX: No QMP command like "info block" which can get the + * block device information yet. + */ This is not correct, it is simply called 'query-block'
Ah, yes, will update.
Regards, Daniel

于 2011年08月12日 21:11, Daniel P. Berrange 写道:
On Fri, Aug 12, 2011 at 09:34:41PM +0800, Osier Yang wrote:
Intorduce new monitor functions to get the media status (ejected or inserted) of removable block device via qemu monitor command "info block".
QEMU upstream will expose the the media status like: cd: type=cdrom removable=1 locked=0 ejected=0
The related patch: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg00408.html Hmm, that patch isn't merged upstream yet. We ought to wait for it to be merged, since historically discussions about this kind of thing on QEMU-devel have been quiet unpredictable.
Although it's unlikely to expose other information of removable block device in future via the new function, returned the info in argument "*ejected" just in case of there is new requirement to expose other info. --- src/qemu/qemu_monitor.c | 20 ++++++++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 14 ++++++ src/qemu/qemu_monitor_json.h | 4 +- src/qemu/qemu_monitor_text.c | 105 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + 6 files changed, 148 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index db6107c..7a41e8f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1227,6 +1227,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; }
+int qemuMonitorGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) The last arg should really be 'bool *ejected'
Oh, missed this, this is left by my previous trying to work out a new public API. will update.
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2a9a078..f59489d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1519,6 +1519,20 @@ cleanup: return ret; }
+int qemuMonitorJSONGetRemovableBlockMediaStatus(qemuMonitorPtr mon, + const char *devname, + unsigned int *ejected) +{ + /* XXX: No QMP command like "info block" which can get the + * block device information yet. + */ This is not correct, it is simply called 'query-block'
Regards, Daniel

This patch is mainly to solve the migration problem: After the media is ejected inside guest, and one removes the source of the media outside, migration will fail early, as we start the qemu daemon on dest host with the source of the removable block still existing, this causes failure when trying to do cgroup setting on the media source then. All the migration functions try to get the domain XML before doing preparation on dest host. Except one specify the domain XML externally (e.g. option "--xml" of "virsh migrate"). So this patch changes the function "qemuDomainFormatXML" to check the media status of removable block using the new introduced monitor functions, and clear the source path if the media ejected, so that it won't start the qemu daemon on dest host along with the media path. Public migration APIs invokes "driver->domainGetXMLDesc"; "driver->domainMigratePeer2Peer" uses "qemuDomainFormatXML" for v2 migration protocol and "qemuDomainBegin" for v3 migration protocol, to get the domain XML before preparation on dest host. All of these 3 functions are based on "qemuDomainFormatXML", that's why making changes on it. The changes affect the running domain config, (e.g. for a running domain, "virsh dumpxml" won't display the source path of the media anymore once it's ejected inside guest. But think it's reasonable to do like so. But the migration will still fail even with this patch when one specified the domain XML externally. However, think this is not what we should care about. It's fault of user? --- src/qemu/qemu_domain.c | 37 +++++++++++++++++++++++++++++++++++++ 1 files changed, 37 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 675c6df..968af69 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1063,12 +1063,49 @@ char *qemuDomainFormatXML(struct qemud_driver *driver, unsigned int flags) { virDomainDefPtr def; + unsigned int ejected = 0; + int err, i; + qemuDomainObjPrivatePtr priv = NULL; + if ((flags & VIR_DOMAIN_XML_INACTIVE) && vm->newDef) def = vm->newDef; else def = vm->def; + /* Check if the media of removable block is ejected, and clear the + * source path if it's ejected. + */ + if (virDomainObjIsActive(vm)) { + priv = vm->privateData; + + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_QUERY) < 0) + return NULL; + + for (i = 0; i < vm->def->ndisks; i++) { + if (vm->def->disks[i]->src && + (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM || + vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)) { + qemuDomainObjEnterMonitorWithDriver(driver, vm); + err = qemuMonitorGetRemovableBlockMediaStatus(priv->mon, + vm->def->disks[i]->info.alias, + &ejected); + qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (err == -1) + return NULL; + + if (ejected == 1) + VIR_FREE(vm->def->disks[i]->src); + } + } + + if (qemuDomainObjEndJob(driver, vm) == 0) { + vm = NULL; + return NULL; + } + } + return qemuDomainDefFormatXML(driver, def, flags); } -- 1.7.6

On Fri, Aug 12, 2011 at 09:34:42PM +0800, Osier Yang wrote:
This patch is mainly to solve the migration problem: After the media is ejected inside guest, and one removes the source of the media outside, migration will fail early, as we start the qemu daemon on dest host with the source of the removable block still existing, this causes failure when trying to do cgroup setting on the media source then.
All the migration functions try to get the domain XML before doing preparation on dest host. Except one specify the domain XML externally (e.g. option "--xml" of "virsh migrate"). So this patch changes the function "qemuDomainFormatXML" to check the media status of removable block using the new introduced monitor functions, and clear the source path if the media ejected, so that it won't start the qemu daemon on dest host along with the media path.
Public migration APIs invokes "driver->domainGetXMLDesc"; "driver->domainMigratePeer2Peer" uses "qemuDomainFormatXML" for v2 migration protocol and "qemuDomainBegin" for v3 migration protocol, to get the domain XML before preparation on dest host. All of these 3 functions are based on "qemuDomainFormatXML", that's why making changes on it.
The changes affect the running domain config, (e.g. for a running domain, "virsh dumpxml" won't display the source path of the media anymore once it's ejected inside guest. But think it's reasonable to do like so.
No, IMHO, we really do not want to be going to the QEMU monitor every time we dump XML. We want the dump XML operation to be fast & reliable even in the face of a stuck/deadlocked QEMU [1]. If we want Dump XML to always show the corect state, then we need QEMU to be giving us an event notification. Daniel [1] Yes we have a problem with the balloon query update in this respect too, but we need to fix that, not make the problem worse. -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

于 2011年08月12日 21:04, Daniel P. Berrange 写道:
On Fri, Aug 12, 2011 at 09:34:42PM +0800, Osier Yang wrote:
This patch is mainly to solve the migration problem: After the media is ejected inside guest, and one removes the source of the media outside, migration will fail early, as we start the qemu daemon on dest host with the source of the removable block still existing, this causes failure when trying to do cgroup setting on the media source then.
All the migration functions try to get the domain XML before doing preparation on dest host. Except one specify the domain XML externally (e.g. option "--xml" of "virsh migrate"). So this patch changes the function "qemuDomainFormatXML" to check the media status of removable block using the new introduced monitor functions, and clear the source path if the media ejected, so that it won't start the qemu daemon on dest host along with the media path.
Public migration APIs invokes "driver->domainGetXMLDesc"; "driver->domainMigratePeer2Peer" uses "qemuDomainFormatXML" for v2 migration protocol and "qemuDomainBegin" for v3 migration protocol, to get the domain XML before preparation on dest host. All of these 3 functions are based on "qemuDomainFormatXML", that's why making changes on it.
The changes affect the running domain config, (e.g. for a running domain, "virsh dumpxml" won't display the source path of the media anymore once it's ejected inside guest. But think it's reasonable to do like so. No, IMHO, we really do not want to be going to the QEMU monitor every time we dump XML. We want the dump XML operation to be fast& reliable even in the face of a stuck/deadlocked QEMU [1].
If we want Dump XML to always show the corect state, then we need QEMU to be giving us an event notification.
Daniel
[1] Yes we have a problem with the balloon query update in this respect too, but we need to fix that, not make the problem worse.
Agree, but there is no better idea in my mind to solve the migration problem with "info block" but no qemu event. Both the public migration APIs and the internal qemu migration APIs need to get the domain XML before doing preparation on dest host, all of them need a way to let the dest known if the disk is ejected. That's why I choosed to change "qemuDomainFormatXML". Or do you have any good idea? Thanks Osier

On Fri, Aug 12, 2011 at 09:14:09PM +0800, Osier Yang wrote:
于 2011年08月12日 21:04, Daniel P. Berrange 写道:
The changes affect the running domain config, (e.g. for a running domain, "virsh dumpxml" won't display the source path of the media anymore once it's ejected inside guest. But think it's reasonable to do like so. No, IMHO, we really do not want to be going to the QEMU monitor every time we dump XML. We want the dump XML operation to be fast& reliable even in the face of a stuck/deadlocked QEMU [1].
If we want Dump XML to always show the corect state, then we need QEMU to be giving us an event notification.
Daniel
[1] Yes we have a problem with the balloon query update in this respect too, but we need to fix that, not make the problem worse.
Agree, but there is no better idea in my mind to solve the migration problem with "info block" but no qemu event.
Both the public migration APIs and the internal qemu migration APIs need to get the domain XML before doing preparation on dest host, all of them need a way to let the dest known if the disk is ejected. That's why I choosed to change "qemuDomainFormatXML".
I see the problem is that with the v2 migration protocol, we simply call into the normal virDomainGetXMLDesc API call. This was fixed in v3 migration to use the virDomainMigrateBegin call to get the XML.
Or do you have any good idea? Thanks
I really think we should invest our time in getting QEMU to provide us with event notifications, rather than trying to hack around it by using info block. It really shouldn't be this hard to get an event from QEMU when they already have the data we need. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

[ CC to Markus and Luiz ] 于 2011年08月12日 22:00, Daniel P. Berrange 写道:
On Fri, Aug 12, 2011 at 09:14:09PM +0800, Osier Yang wrote:
The changes affect the running domain config, (e.g. for a running domain, "virsh dumpxml" won't display the source path of the media anymore once it's ejected inside guest. But think it's reasonable to do like so. No, IMHO, we really do not want to be going to the QEMU monitor every time we dump XML. We want the dump XML operation to be fast& reliable even in the face of a stuck/deadlocked QEMU [1].
If we want Dump XML to always show the corect state, then we need QEMU to be giving us an event notification.
Daniel
[1] Yes we have a problem with the balloon query update in this respect too, but we need to fix that, not make the problem worse. Agree, but there is no better idea in my mind to solve the migration
于 2011年08月12日 21:04, Daniel P. Berrange 写道: problem with "info block" but no qemu event.
Both the public migration APIs and the internal qemu migration APIs need to get the domain XML before doing preparation on dest host, all of them need a way to let the dest known if the disk is ejected. That's why I choosed to change "qemuDomainFormatXML". I see the problem is that with the v2 migration protocol, we simply call into the normal virDomainGetXMLDesc API call. This was fixed in v3 migration to use the virDomainMigrateBegin call to get the XML.
Or do you have any good idea? Thanks I really think we should invest our time in getting QEMU to provide us with event notifications, rather than trying to hack around it by using info block. It really shouldn't be this hard to get an event from QEMU when they already have the data we need.
Daniel
Hi Markus and Luiz, As the discussion in this thread, we have problem on solving the problem with just "info block" but no event. Do you think it's acceptable for qemu upstream to add the event support? I can invest time on doing the work if need. Thanks Osier
participants (2)
-
Daniel P. Berrange
-
Osier Yang