6:20 a.m.
The first patch is bare bug fix (without any bug reported though).
The second is again a bug fix, but not so easy to spot. Basically, when we
change a media, we should issue 'eject' first, then wait until the tray gets
open, after which we can finally issue 'change' command. Even for bare
'eject'
we ought to check status, shouldn't we?
Michal Privoznik (2):
qemu_monitor: Fix tray-open attribute in query-block
qemu_hotplug: Rework media changing process
src/qemu/qemu_hotplug.c | 51 +++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 6 +++---
3 files changed, 52 insertions(+), 7 deletions(-)
--
1.8.0.2
6:20 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
With our code, we fail to query for tray-open attribute currently.
That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
It always has been. However, we got it exactly the opposite.
---
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index de5f115..0aa3bcb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
/* Don't check for success here, because 'tray-open' is presented
iff
* medium is ejected.
*/
- ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
+ ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
&info->tray_open));
/* Missing io-status indicates no error */
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 6506f9d..bc0a11d 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -809,10 +809,10 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon,
VIR_DEBUG("error reading locked: %s", p);
else
info->locked = (tmp != 0);
- } else if (STRPREFIX(p, "tray_open=")) {
- p += strlen("tray_open=");
+ } else if (STRPREFIX(p, "tray-open=")) {
+ p += strlen("tray-open=");
if (virStrToLong_i(p, &dummy, 10, &tmp) == -1)
- VIR_DEBUG("error reading tray_open: %s", p);
+ VIR_DEBUG("error reading tray-open: %s", p);
else
info->tray_open = (tmp != 0);
} else if (STRPREFIX(p, "io-status=")) {
--
1.8.0.2
7:05 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
On 01/25/13 13:20, Michal Privoznik wrote:
With our code, we fail to query for tray-open attribute currently.
That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
It always has been. However, we got it exactly the opposite.
---
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index de5f115..0aa3bcb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
/* Don't check for success here, because 'tray-open' is presented
iff
* medium is ejected.
*/
- ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
+ ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
&info->tray_open));
Indeed. The returned value is with the underscore, while the
documentation uses the dash.
/* Missing io-status indicates no error */
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 6506f9d..bc0a11d 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
+ } else if (STRPREFIX(p, "tray-open=")) {
This too matches the reality.
ACK. I think it would be worth notifying the qemu folks as the docs are
mistaken.
Peter
7:31 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
On 01/25/2013 06:05 AM, Peter Krempa wrote:
On 01/25/13 13:20, Michal Privoznik wrote:
> With our code, we fail to query for tray-open attribute currently.
> That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
> It always has been. However, we got it exactly the opposite.
> ---
> src/qemu/qemu_monitor_json.c | 2 +-
> src/qemu/qemu_monitor_text.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index de5f115..0aa3bcb 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> /* Don't check for success here, because 'tray-open' is
> presented iff
> * medium is ejected.
> */
> - ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
> + ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
> &info->tray_open));
Indeed. The returned value is with the underscore, while the
documentation uses the dash.
Where did you find that? Because qemu.git/qapi-schema.json says:
{ 'type': 'BlockInfo',
'data': {'device': 'str', 'type': 'str',
'removable': 'bool',
'locked': 'bool', '*inserted':
'BlockDeviceInfo',
'*tray_open': 'bool', '*io-status':
'BlockDeviceIoStatus',
'*dirty': 'BlockDirtyInfo' } }
with the underscore, and that's the file I refer to for documentation of
QMP.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:33 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
On 25.01.2013 14:31, Eric Blake wrote:
On 01/25/2013 06:05 AM, Peter Krempa wrote:
> On 01/25/13 13:20, Michal Privoznik wrote:
>> With our code, we fail to query for tray-open attribute currently.
>> That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
>> It always has been. However, we got it exactly the opposite.
>> ---
>> src/qemu/qemu_monitor_json.c | 2 +-
>> src/qemu/qemu_monitor_text.c | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index de5f115..0aa3bcb 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>> /* Don't check for success here, because 'tray-open' is
>> presented iff
>> * medium is ejected.
>> */
>> - ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
>> + ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
>> &info->tray_open));
>
> Indeed. The returned value is with the underscore, while the
> documentation uses the dash.
Where did you find that? Because qemu.git/qapi-schema.json says:
{ 'type': 'BlockInfo',
'data': {'device': 'str', 'type': 'str',
'removable': 'bool',
'locked': 'bool', '*inserted':
'BlockDeviceInfo',
'*tray_open': 'bool', '*io-status':
'BlockDeviceIoStatus',
'*dirty': 'BlockDirtyInfo' } }
with the underscore, and that's the file I refer to for documentation of
QMP.
qmp-commands.hx:- "tray-open": only present if removable, true if the
device has a tray,
7:35 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
On 01/25/13 14:31, Eric Blake wrote:
On 01/25/2013 06:05 AM, Peter Krempa wrote:
> On 01/25/13 13:20, Michal Privoznik wrote:
>> With our code, we fail to query for tray-open attribute currently.
>> That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
>> It always has been. However, we got it exactly the opposite.
>> ---
>> src/qemu/qemu_monitor_json.c | 2 +-
>> src/qemu/qemu_monitor_text.c | 6 +++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index de5f115..0aa3bcb 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>> /* Don't check for success here, because 'tray-open' is
>> presented iff
>> * medium is ejected.
>> */
>> - ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
>> + ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
>> &info->tray_open));
>
> Indeed. The returned value is with the underscore, while the
> documentation uses the dash.
Where did you find that? Because qemu.git/qapi-schema.json says:
{ 'type': 'BlockInfo',
'data': {'device': 'str', 'type': 'str',
'removable': 'bool',
'locked': 'bool', '*inserted':
'BlockDeviceInfo',
'*tray_open': 'bool', '*io-status':
'BlockDeviceIoStatus',
'*dirty': 'BlockDirtyInfo' } }
with the underscore, and that's the file I refer to for documentation of
QMP.
I usualy refer to qemu.git/qmp-commands.hx:
query-block
-----------
Show the block devices.
Each block device information is stored in a json-object and the returned value
is a json-array of all devices.
Each json-object contain the following:
- "device": device name (json-string)
- "type": device type (json-string)
- deprecated, retained for backward compatibility
- Possible values: "unknown"
- "removable": true if the device is removable, false otherwise (json-bool)
- "locked": true if the device is locked, false otherwise (json-bool)
- "tray-open": only present if removable, true if the device has a tray,
and it is open (json-bool)
- "inserted": only present if the device is inserted, it is a json-object
containing the following:
[...]
7:42 a.m.
New subject: [libvirt] [PATCH 1/2] qemu_monitor: Fix tray-open attribute in query-block
On 25.01.2013 14:05, Peter Krempa wrote:
On 01/25/13 13:20, Michal Privoznik wrote:
> With our code, we fail to query for tray-open attribute currently.
> That's because in HMP it is 'tray-open' and in QMP it's
'tray_open'.
> It always has been. However, we got it exactly the opposite.
> ---
> src/qemu/qemu_monitor_json.c | 2 +-
> src/qemu/qemu_monitor_text.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index de5f115..0aa3bcb 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1588,7 +1588,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
> /* Don't check for success here, because 'tray-open' is
1: ^^
> presented iff
> * medium is ejected.
> */
> - ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open",
> + ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open",
> &info->tray_open));
Indeed. The returned value is with the underscore, while the
documentation uses the dash.
>
> /* Missing io-status indicates no error */
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 6506f9d..bc0a11d 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> + } else if (STRPREFIX(p, "tray-open=")) {
This too matches the reality.
ACK. I think it would be worth notifying the qemu folks as the docs are
mistaken.
Peter
Thanks. I've pushed this one since this is pure bug fix unrelated to the
latter patch of mine. I've sent patch to qemu devel list as well:
http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg04804.html
And I've updated the forgot comment [1] as well.
Michal
6:20 a.m.
New subject: [libvirt] [PATCH 2/2] qemu_hotplug: Rework media changing process
https://bugzilla.redhat.com/show_bug.cgi?id=892289
It seems like with new udev within guest OS, the tray is locked,
so we need to:
- 'eject'
- wait shortly
- 'change'
However, the 'wait shortly' step is better to be substituted with
active polling on tray_open attribute on the device. Moreover,
even when doing bare 'eject', we should check for 'tray_open' as
guest may have locked the tray.
---
src/qemu/qemu_hotplug.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8103183..f373c44 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -48,6 +48,7 @@
#include "virstoragefile.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
+#define CHANGE_MEDIA_RETRIES 10
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -59,6 +60,10 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
int ret;
char *driveAlias = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virHashTablePtr table = NULL;
+ struct qemuDomainDiskInfo *info = NULL;
+ int retries = CHANGE_MEDIA_RETRIES;
+ virErrorPtr origError = NULL;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (vm->def->disks[i]->bus == disk->bus &&
@@ -86,7 +91,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Removable media not supported for %s device"),
- virDomainDiskDeviceTypeToString(disk->device));
+ virDomainDiskDeviceTypeToString(disk->device));
return -1;
}
@@ -105,8 +110,45 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto error;
qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+
+ /* we don't want to report errors from media tray_open polling */
+ origError = virSaveLastError();
+ while (retries--) {
+ virHashFree(table);
+ table = qemuMonitorGetBlockInfo(priv->mon);
+ if (!table)
+ goto exit_monitor;
+
+ info = qemuMonitorBlockInfoLookup(table, origdisk->info.alias);
+ if (!info) {
+ virHashFree(table);
+ goto exit_monitor;
+ }
+
+ if (info->tray_open)
+ break;
+
+ usleep(200 * 1000); /* sleep 200ms */
+ }
+ virHashFree(table);
+
if (disk->src) {
+ /* deliberately don't depend on 'ret' as 'eject' may have
failed for the
+ * fist time and we are gonna check the drive state anyway */
const char *format = NULL;
+
+ /* We haven't succeeded yet */
+ ret = -1;
+
+ if (retries <= 0) {
+ virFreeError(origError);
+ origError = NULL;
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Unable to eject media before changing it"));
+ goto exit_monitor;
+ }
+
if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) {
if (disk->format > 0)
format = virStorageFileFormatTypeToString(disk->format);
@@ -116,8 +158,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
ret = qemuMonitorChangeMedia(priv->mon,
driveAlias,
disk->src, format);
- } else {
- ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+ }
+exit_monitor:
+ if (origError) {
+ virSetError(origError);
+ virFreeError(origError);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
--
1.8.0.2
6:32 a.m.
New subject: [libvirt] [PATCH 2/2] qemu_hotplug: Rework media changing process
On Fri, Jan 25, 2013 at 01:20:43PM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=892289
It seems like with new udev within guest OS, the tray is locked,
so we need to:
- 'eject'
- wait shortly
- 'change'
However, the 'wait shortly' step is better to be substituted with
active polling on tray_open attribute on the device. Moreover,
even when doing bare 'eject', we should check for 'tray_open' as
guest may have locked the tray.
QEMU emits an event when the tray state changes, so we should
just be listening for that. In the case where we have an old
QEMU lacking the tray state event, then we're also missing
the tray state in the block info query, so no worse off. So
just skip the polling and wait for the event to arrive.
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 :|
8:53 a.m.
New subject: [libvirt] [PATCH v2] qemu_hotplug: Rework media changing process
https://bugzilla.redhat.com/show_bug.cgi?id=892289
It seems like with new udev within guest OS, the tray is locked,
so we need to:
- 'eject'
- wait for tray to open
- 'change'
Moreover, even when doing bare 'eject', we should check for
'tray_open' as guest may have locked the tray. However, the
waiting phase shouldn't be unbounded, so I've chosen 10 retries
maximum, each per 500ms. This should give enough time for guest
to eject a media and open the tray.
---
diff to v1:
- listen to TRAY event instead of active polling
src/qemu/qemu_hotplug.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8103183..18c4109 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -48,6 +48,7 @@
#include "virstoragefile.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
+#define CHANGE_MEDIA_RETRIES 10
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -59,6 +60,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
int ret;
char *driveAlias = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ int retries = CHANGE_MEDIA_RETRIES;
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (vm->def->disks[i]->bus == disk->bus &&
@@ -86,7 +88,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
origdisk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Removable media not supported for %s device"),
- virDomainDiskDeviceTypeToString(disk->device));
+ virDomainDiskDeviceTypeToString(disk->device));
return -1;
}
@@ -105,8 +107,31 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto error;
qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+
+ /* we don't want to report errors from media tray_open polling */
+ while (retries--) {
+ if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
+ break;
+
+ VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d", retries);
+ usleep(500 * 1000); /* sleep 500ms */
+ }
+
if (disk->src) {
+ /* deliberately don't depend on 'ret' as 'eject' may have
failed for the
+ * fist time and we are gonna check the drive state anyway */
const char *format = NULL;
+
+ /* We haven't succeeded yet */
+ ret = -1;
+
+ if (retries <= 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Unable to eject media before changing it"));
+ goto exit_monitor;
+ }
+
if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) {
if (disk->format > 0)
format = virStorageFileFormatTypeToString(disk->format);
@@ -116,9 +141,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
ret = qemuMonitorChangeMedia(priv->mon,
driveAlias,
disk->src, format);
- } else {
- ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
}
+exit_monitor:
qemuDomainObjExitMonitorWithDriver(driver, vm);
virDomainAuditDisk(vm, origdisk->src, disk->src, "update", ret >=
0);
--
1.8.0.2
5:07 p.m.
New subject: [libvirt] [PATCH v2] qemu_hotplug: Rework media changing process
On 01/25/2013 07:53 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=892289
It seems like with new udev within guest OS, the tray is locked,
so we need to:
- 'eject'
- wait for tray to open
- 'change'
Moreover, even when doing bare 'eject', we should check for
'tray_open' as guest may have locked the tray. However, the
waiting phase shouldn't be unbounded, so I've chosen 10 retries
maximum, each per 500ms. This should give enough time for guest
to eject a media and open the tray.
---
diff to v1:
- listen to TRAY event instead of active polling
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:04 a.m.
New subject: [libvirt] [PATCH v2] qemu_hotplug: Rework media changing process
On 26.01.2013 00:07, Eric Blake wrote:
On 01/25/2013 07:53 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>
> It seems like with new udev within guest OS, the tray is locked,
> so we need to:
> - 'eject'
> - wait for tray to open
> - 'change'
>
> Moreover, even when doing bare 'eject', we should check for
> 'tray_open' as guest may have locked the tray. However, the
> waiting phase shouldn't be unbounded, so I've chosen 10 retries
> maximum, each per 500ms. This should give enough time for guest
> to eject a media and open the tray.
> ---
>
> diff to v1:
> - listen to TRAY event instead of active polling
>
ACK.
Thanks, pushed.
Michal
12:47 a.m.
Hi Michal, Daniel
It seems there is something wrong with the 2/2 part of this patchset.
When I do an "change-media" command in virsh, it doesn't do any better
than before, but even worse(I must to wait 5 secs to see the error).
I'm not family with libvirt, just add some log things in the qemu_hotplug.c
and found the tray_status never change to open, but with michal's original
patch (which do active poll on tray_status), I can do "change-media"
successfully
every time.
IMHO, there must be some bug in the processing of qemu tray change.
Thanks,
Wei
On 01/25/2013 08:20 PM, Michal Privoznik wrote:
The first patch is bare bug fix (without any bug reported though).
The second is again a bug fix, but not so easy to spot. Basically, when we
change a media, we should issue 'eject' first, then wait until the tray gets
open, after which we can finally issue 'change' command. Even for bare
'eject'
we ought to check status, shouldn't we?
Michal Privoznik (2):
qemu_monitor: Fix tray-open attribute in query-block
qemu_hotplug: Rework media changing process
src/qemu/qemu_hotplug.c | 51 +++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 6 +++---
3 files changed, 52 insertions(+), 7 deletions(-)
3:07 a.m.
On 17.05.2013 07:47, Li Wei wrote:
Hi Michal, Daniel
It seems there is something wrong with the 2/2 part of this patchset.
When I do an "change-media" command in virsh, it doesn't do any better
than before, but even worse(I must to wait 5 secs to see the error).
I'm not family with libvirt, just add some log things in the qemu_hotplug.c
and found the tray_status never change to open, but with michal's original
patch (which do active poll on tray_status), I can do "change-media"
successfully
every time.
Yes & no. Thing is - originally we ignored tray status in the guest. So
in case guest has locked the tray and thus ignored our eject request, we
have changed the media anyway, resulting in possible I/O errors within
guest.
With my patch, we send eject request to the qemu, and actively poll for
the tray to open. We check for up to 10 times every 200ms. So guest has
2 seconds to open the tray. If the guest doesn't eject media and open
the tray we don't proceed with forcibly changing the media.
These patches are actually a bug fix for
https://bugzilla.redhat.com/show_bug.cgi?id=892289
IMHO, there must be some bug in the processing of qemu tray change.
Thanks,
Wei
On 01/25/2013 08:20 PM, Michal Privoznik wrote:
> The first patch is bare bug fix (without any bug reported though).
> The second is again a bug fix, but not so easy to spot. Basically, when we
> change a media, we should issue 'eject' first, then wait until the tray gets
> open, after which we can finally issue 'change' command. Even for bare
'eject'
> we ought to check status, shouldn't we?
>
> Michal Privoznik (2):
> qemu_monitor: Fix tray-open attribute in query-block
> qemu_hotplug: Rework media changing process
>
> src/qemu/qemu_hotplug.c | 51 +++++++++++++++++++++++++++++++++++++++++---
> src/qemu/qemu_monitor_json.c | 2 +-
> src/qemu/qemu_monitor_text.c | 6 +++---
> 3 files changed, 52 insertions(+), 7 deletions(-)
>
3:16 a.m.
On 05/17/2013 04:07 PM, Michal Privoznik wrote:
On 17.05.2013 07:47, Li Wei wrote:
> Hi Michal, Daniel
>
> It seems there is something wrong with the 2/2 part of this patchset.
> When I do an "change-media" command in virsh, it doesn't do any better
> than before, but even worse(I must to wait 5 secs to see the error).
>
> I'm not family with libvirt, just add some log things in the qemu_hotplug.c
> and found the tray_status never change to open, but with michal's original
> patch (which do active poll on tray_status), I can do "change-media"
successfully
> every time.
Yes & no. Thing is - originally we ignored tray status in the guest. So
in case guest has locked the tray and thus ignored our eject request, we
have changed the media anyway, resulting in possible I/O errors within
guest.
With my patch, we send eject request to the qemu, and actively poll for
the tray to open. We check for up to 10 times every 200ms. So guest has
2 seconds to open the tray. If the guest doesn't eject media and open
the tray we don't proceed with forcibly changing the media.
These patches are actually a bug fix for
https://bugzilla.redhat.com/show_bug.cgi?id=892289
Hmm... I think you misunderstood my meaning(sorry for my pool english),
simply to say, I can get expected behavior by apply patch V1 but not V2.
In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN but
this never happened.
Thanks,
Wei
>
> IMHO, there must be some bug in the processing of qemu tray change.
>
> Thanks,
> Wei
>
>
> On 01/25/2013 08:20 PM, Michal Privoznik wrote:
>> The first patch is bare bug fix (without any bug reported though).
>> The second is again a bug fix, but not so easy to spot. Basically, when we
>> change a media, we should issue 'eject' first, then wait until the tray
gets
>> open, after which we can finally issue 'change' command. Even for bare
'eject'
>> we ought to check status, shouldn't we?
>>
>> Michal Privoznik (2):
>> qemu_monitor: Fix tray-open attribute in query-block
>> qemu_hotplug: Rework media changing process
>>
>> src/qemu/qemu_hotplug.c | 51 +++++++++++++++++++++++++++++++++++++++++---
>> src/qemu/qemu_monitor_json.c | 2 +-
>> src/qemu/qemu_monitor_text.c | 6 +++---
>> 3 files changed, 52 insertions(+), 7 deletions(-)
>>
3:52 a.m.
On 17.05.2013 10:16, Li Wei wrote:
On 05/17/2013 04:07 PM, Michal Privoznik wrote:
> On 17.05.2013 07:47, Li Wei wrote:
>> Hi Michal, Daniel
>>
>> It seems there is something wrong with the 2/2 part of this patchset.
>> When I do an "change-media" command in virsh, it doesn't do any
better
>> than before, but even worse(I must to wait 5 secs to see the error).
>>
>> I'm not family with libvirt, just add some log things in the qemu_hotplug.c
>> and found the tray_status never change to open, but with michal's original
>> patch (which do active poll on tray_status), I can do "change-media"
successfully
>> every time.
>
> Yes & no. Thing is - originally we ignored tray status in the guest. So
> in case guest has locked the tray and thus ignored our eject request, we
> have changed the media anyway, resulting in possible I/O errors within
> guest.
>
> With my patch, we send eject request to the qemu, and actively poll for
> the tray to open. We check for up to 10 times every 200ms. So guest has
> 2 seconds to open the tray. If the guest doesn't eject media and open
> the tray we don't proceed with forcibly changing the media.
>
> These patches are actually a bug fix for
> https://bugzilla.redhat.com/show_bug.cgi?id=892289
Hmm... I think you misunderstood my meaning(sorry for my pool english),
simply to say, I can get expected behavior by apply patch V1 but not V2.
In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN but
this never happened.
Thanks,
Wei
Aaah, now I understand. So you are saying that the event is not being
delivered. What's the qemu version?
If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
http://wiki.libvirt.org/page/DebugLogs
Or maybe the timeout is just short for your guest to eject the tray. So
after a while, does 'virsh change-media' succeed?
Michal
>
>>
>> IMHO, there must be some bug in the processing of qemu tray change.
>>
>> Thanks,
>> Wei
>>
>>
>> On 01/25/2013 08:20 PM, Michal Privoznik wrote:
>>> The first patch is bare bug fix (without any bug reported though).
>>> The second is again a bug fix, but not so easy to spot. Basically, when we
>>> change a media, we should issue 'eject' first, then wait until the
tray gets
>>> open, after which we can finally issue 'change' command. Even for
bare 'eject'
>>> we ought to check status, shouldn't we?
>>>
>>> Michal Privoznik (2):
>>> qemu_monitor: Fix tray-open attribute in query-block
>>> qemu_hotplug: Rework media changing process
>>>
>>> src/qemu/qemu_hotplug.c | 51
+++++++++++++++++++++++++++++++++++++++++---
>>> src/qemu/qemu_monitor_json.c | 2 +-
>>> src/qemu/qemu_monitor_text.c | 6 +++---
>>> 3 files changed, 52 insertions(+), 7 deletions(-)
>>>
>
>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:37 a.m.
On 05/17/2013 04:52 PM, Michal Privoznik wrote:
On 17.05.2013 10:16, Li Wei wrote:
> On 05/17/2013 04:07 PM, Michal Privoznik wrote:
>> On 17.05.2013 07:47, Li Wei wrote:
>>> Hi Michal, Daniel
>>>
>>> It seems there is something wrong with the 2/2 part of this patchset.
>>> When I do an "change-media" command in virsh, it doesn't do any
better
>>> than before, but even worse(I must to wait 5 secs to see the error).
>>>
>>> I'm not family with libvirt, just add some log things in the
qemu_hotplug.c
>>> and found the tray_status never change to open, but with michal's
original
>>> patch (which do active poll on tray_status), I can do
"change-media" successfully
>>> every time.
>>
>> Yes & no. Thing is - originally we ignored tray status in the guest. So
>> in case guest has locked the tray and thus ignored our eject request, we
>> have changed the media anyway, resulting in possible I/O errors within
>> guest.
>>
>> With my patch, we send eject request to the qemu, and actively poll for
>> the tray to open. We check for up to 10 times every 200ms. So guest has
>> 2 seconds to open the tray. If the guest doesn't eject media and open
>> the tray we don't proceed with forcibly changing the media.
>>
>> These patches are actually a bug fix for
>> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>
> Hmm... I think you misunderstood my meaning(sorry for my pool english),
> simply to say, I can get expected behavior by apply patch V1 but not V2.
>
> In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
> wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN but
> this never happened.
>
> Thanks,
> Wei
Aaah, now I understand. So you are saying that the event is not being
delivered. What's the qemu version?
I have the following version:
virsh # version
Compiled against library: libvirt 1.0.5
Using library: libvirt 1.0.5
Using API: QEMU 1.0.5
Running hypervisor: QEMU 1.4.1
If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
http://wiki.libvirt.org/page/DebugLogs
Or maybe the timeout is just short for your guest to eject the tray. So
after a while, does 'virsh change-media' succeed?
Yes, I can see the DEVICE_TRAY_MOVED in the log file, it goes after
the debug message "qemuDomainChangeEjectableMedia:103 : Waiting 500ms for tray to
open. Retries left 0".
does that mean the timeout is too short?
If the guest needs too much time to eject, I'm wondering why when I use
the v1 patch, I can get succeed in about 1 to 2 seconds. I doubt if there
are other problems cause tray_status not updated in time?
I appended the log file for your reference, if you need any other information,
please tell me.
Thanks,
Wei
Michal
>
>>
>>>
>>> IMHO, there must be some bug in the processing of qemu tray change.
>>>
>>> Thanks,
>>> Wei
>>>
>>>
>>> On 01/25/2013 08:20 PM, Michal Privoznik wrote:
>>>> The first patch is bare bug fix (without any bug reported though).
>>>> The second is again a bug fix, but not so easy to spot. Basically, when
we
>>>> change a media, we should issue 'eject' first, then wait until
the tray gets
>>>> open, after which we can finally issue 'change' command. Even for
bare 'eject'
>>>> we ought to check status, shouldn't we?
>>>>
>>>> Michal Privoznik (2):
>>>> qemu_monitor: Fix tray-open attribute in query-block
>>>> qemu_hotplug: Rework media changing process
>>>>
>>>> src/qemu/qemu_hotplug.c | 51
+++++++++++++++++++++++++++++++++++++++++---
>>>> src/qemu/qemu_monitor_json.c | 2 +-
>>>> src/qemu/qemu_monitor_text.c | 6 +++---
>>>> 3 files changed, 52 insertions(+), 7 deletions(-)
>>>>
>>
>>
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
5:33 a.m.
On 17.05.2013 11:37, Li Wei wrote:
On 05/17/2013 04:52 PM, Michal Privoznik wrote:
> On 17.05.2013 10:16, Li Wei wrote:
>> On 05/17/2013 04:07 PM, Michal Privoznik wrote:
>>> On 17.05.2013 07:47, Li Wei wrote:
>>>> Hi Michal, Daniel
>>>>
>>>> It seems there is something wrong with the 2/2 part of this patchset.
>>>> When I do an "change-media" command in virsh, it doesn't do
any better
>>>> than before, but even worse(I must to wait 5 secs to see the error).
>>>>
>>>> I'm not family with libvirt, just add some log things in the
qemu_hotplug.c
>>>> and found the tray_status never change to open, but with michal's
original
>>>> patch (which do active poll on tray_status), I can do
"change-media" successfully
>>>> every time.
>>>
>>> Yes & no. Thing is - originally we ignored tray status in the guest. So
>>> in case guest has locked the tray and thus ignored our eject request, we
>>> have changed the media anyway, resulting in possible I/O errors within
>>> guest.
>>>
>>> With my patch, we send eject request to the qemu, and actively poll for
>>> the tray to open. We check for up to 10 times every 200ms. So guest has
>>> 2 seconds to open the tray. If the guest doesn't eject media and open
>>> the tray we don't proceed with forcibly changing the media.
>>>
>>> These patches are actually a bug fix for
>>> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>>
>> Hmm... I think you misunderstood my meaning(sorry for my pool english),
>> simply to say, I can get expected behavior by apply patch V1 but not V2.
>>
>> In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
>> wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN but
>> this never happened.
>>
>> Thanks,
>> Wei
>
> Aaah, now I understand. So you are saying that the event is not being
> delivered. What's the qemu version?
I have the following version:
virsh # version
Compiled against library: libvirt 1.0.5
Using library: libvirt 1.0.5
Using API: QEMU 1.0.5
Running hypervisor: QEMU 1.4.1
>
> If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
> http://wiki.libvirt.org/page/DebugLogs
>
> Or maybe the timeout is just short for your guest to eject the tray. So
> after a while, does 'virsh change-media' succeed?
Yes, I can see the DEVICE_TRAY_MOVED in the log file, it goes after
the debug message "qemuDomainChangeEjectableMedia:103 : Waiting 500ms for tray to
open. Retries left 0".
does that mean the timeout is too short?
If the guest needs too much time to eject, I'm wondering why when I use
the v1 patch, I can get succeed in about 1 to 2 seconds. I doubt if there
are other problems cause tray_status not updated in time?
I appended the log file for your reference, if you need any other information,
please tell me.
Thanks,
Wei
I am pasting some interesting knobs from the log:
2013-05-17 09:04:46.234+0000: 27004: debug : qemuMonitorSend:892 : QEMU_MONITOR_SEND_MSG:
mon=0x7f7ac4000bd0
msg={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
fd=-1
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOWrite:458 :
QEMU_MONITOR_IO_WRITE: mon=0x7f7ac4000bd0
buf={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
len=92 ret=92 errno=2
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"id": "libvirt-7",
"error": {"class": "GenericError", "desc":
"Device 'drive-ide0-1-0' is locked"}}
len=102
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.235+0000: 27004: error : qemuMonitorJSONCheckError:352 : internal
error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is
locked
2013-05-17 09:04:46.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 9
2013-05-17 09:04:46.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 8
2013-05-17 09:04:47.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 7
2013-05-17 09:04:47.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 6
2013-05-17 09:04:48.186+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:48.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 5
2013-05-17 09:04:48.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 4
2013-05-17 09:04:49.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 3
2013-05-17 09:04:49.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 2
2013-05-17 09:04:50.236+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 1
2013-05-17 09:04:50.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 0
2013-05-17 09:04:51.236+0000: 27004: error : qemuDomainChangeEjectableMedia:117 :
operation failed: Unable to eject media before changing it
2013-05-17 09:04:51.236+0000: 27004: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockDiskDetach:282 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 disk=0x7f7ac4003690
2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockManagerNew:128 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 withResources=0
2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerPluginGetDriver:295 :
plugin=0x7f7acc007c00
2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerNew:319 : driver=0x7f7ae53dc5e0
type=0 nparams=5 params=0x7f7ad7d25830 flags=0
2013-05-17 09:04:51.236+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"timestamp":
{"seconds": 1368781488, "microseconds": 186399}, "event":
"DEVICE_TRAY_MOVED", "data": {"device":
"drive-ide0-1-0", "tray-open": true}}
len=151
What we can see here is:
1) libvirt asks qemu to eject
2) qemu responds with "Device is locked" - which is correct as it emulates guest
request
3) However, after a while, the guest decides it's now safe to eject the media (e.g.
all reads have finished, and the device has been unmounted) which is done
4) [not in the pasted log] After a while, you've decided to try again and this time it
succeeded, as the tray is already opened.
What we may do here is lifting the number of retries up. You can try yourself:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d037c9d..ddd8d17 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -50,7 +50,7 @@
#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
-#define CHANGE_MEDIA_RETRIES 10
+#define CHANGE_MEDIA_RETRIES 20
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
Michal
10:31 p.m.
On 05/17, Michal Privoznik wrote:
On 17.05.2013 11:37, Li Wei wrote:
> On 05/17/2013 04:52 PM, Michal Privoznik wrote:
>> On 17.05.2013 10:16, Li Wei wrote:
>>> On 05/17/2013 04:07 PM, Michal Privoznik wrote:
>>>> On 17.05.2013 07:47, Li Wei wrote:
>>>>> Hi Michal, Daniel
>>>>>
>>>>> It seems there is something wrong with the 2/2 part of this
patchset.
>>>>> When I do an "change-media" command in virsh, it
doesn't do any better
>>>>> than before, but even worse(I must to wait 5 secs to see the
error).
>>>>>
>>>>> I'm not family with libvirt, just add some log things in the
qemu_hotplug.c
>>>>> and found the tray_status never change to open, but with
michal's original
>>>>> patch (which do active poll on tray_status), I can do
"change-media" successfully
>>>>> every time.
>>>>
>>>> Yes & no. Thing is - originally we ignored tray status in the guest.
So
>>>> in case guest has locked the tray and thus ignored our eject request,
we
>>>> have changed the media anyway, resulting in possible I/O errors within
>>>> guest.
>>>>
>>>> With my patch, we send eject request to the qemu, and actively poll for
>>>> the tray to open. We check for up to 10 times every 200ms. So guest has
>>>> 2 seconds to open the tray. If the guest doesn't eject media and
open
>>>> the tray we don't proceed with forcibly changing the media.
>>>>
>>>> These patches are actually a bug fix for
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>>>
>>> Hmm... I think you misunderstood my meaning(sorry for my pool english),
>>> simply to say, I can get expected behavior by apply patch V1 but not V2.
>>>
>>> In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
>>> wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN but
>>> this never happened.
>>>
>>> Thanks,
>>> Wei
>>
>> Aaah, now I understand. So you are saying that the event is not being
>> delivered. What's the qemu version?
>
>
> I have the following version:
>
> virsh # version
> Compiled against library: libvirt 1.0.5
> Using library: libvirt 1.0.5
> Using API: QEMU 1.0.5
> Running hypervisor: QEMU 1.4.1
>
>>
>> If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
>> http://wiki.libvirt.org/page/DebugLogs
>>
>> Or maybe the timeout is just short for your guest to eject the tray. So
>> after a while, does 'virsh change-media' succeed?
>
> Yes, I can see the DEVICE_TRAY_MOVED in the log file, it goes after
> the debug message "qemuDomainChangeEjectableMedia:103 : Waiting 500ms for tray
to open. Retries left 0".
> does that mean the timeout is too short?
>
> If the guest needs too much time to eject, I'm wondering why when I use
> the v1 patch, I can get succeed in about 1 to 2 seconds. I doubt if there
> are other problems cause tray_status not updated in time?
>
> I appended the log file for your reference, if you need any other information,
> please tell me.
>
> Thanks,
> Wei
I am pasting some interesting knobs from the log:
2013-05-17 09:04:46.234+0000: 27004: debug : qemuMonitorSend:892 : QEMU_MONITOR_SEND_MSG:
mon=0x7f7ac4000bd0
msg={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
fd=-1
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOWrite:458 :
QEMU_MONITOR_IO_WRITE: mon=0x7f7ac4000bd0
buf={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
len=92 ret=92 errno=2
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"id": "libvirt-7",
"error": {"class": "GenericError", "desc":
"Device 'drive-ide0-1-0' is locked"}}
len=102
2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:46.235+0000: 27004: error : qemuMonitorJSONCheckError:352 : internal
error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is
locked
2013-05-17 09:04:46.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 9
2013-05-17 09:04:46.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 8
2013-05-17 09:04:47.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 7
2013-05-17 09:04:47.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 6
2013-05-17 09:04:48.186+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:48.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 5
2013-05-17 09:04:48.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 4
2013-05-17 09:04:49.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 3
2013-05-17 09:04:49.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 2
2013-05-17 09:04:50.236+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 1
2013-05-17 09:04:50.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 : Waiting
500ms for tray to open. Retries left 0
2013-05-17 09:04:51.236+0000: 27004: error : qemuDomainChangeEjectableMedia:117 :
operation failed: Unable to eject media before changing it
2013-05-17 09:04:51.236+0000: 27004: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockDiskDetach:282 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 disk=0x7f7ac4003690
2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockManagerNew:128 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 withResources=0
2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerPluginGetDriver:295 :
plugin=0x7f7acc007c00
2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerNew:319 :
driver=0x7f7ae53dc5e0 type=0 nparams=5 params=0x7f7ad7d25830 flags=0
2013-05-17 09:04:51.236+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"timestamp":
{"seconds": 1368781488, "microseconds": 186399}, "event":
"DEVICE_TRAY_MOVED", "data": {"device":
"drive-ide0-1-0", "tray-open": true}}
len=151
What we can see here is:
1) libvirt asks qemu to eject
2) qemu responds with "Device is locked" - which is correct as it emulates
guest request
3) However, after a while, the guest decides it's now safe to eject the media (e.g.
all reads have finished, and the device has been unmounted) which is done
4) [not in the pasted log] After a while, you've decided to try again and this time
it succeeded, as the tray is already opened.
What we may do here is lifting the number of retries up. You can try yourself:
I have indeed increased retries to 20 and unfortunately that didn't work either,
we can see in the log file that the DEVICE_TRAY_MOVED event received immediately
after the final retry. After we doubled the retries, that would be more that enough
time for the guest to eject the tray. So, maybe there is something wrong otherwhere.
Thanks,
Wei
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d037c9d..ddd8d17 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -50,7 +50,7 @@
#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
-#define CHANGE_MEDIA_RETRIES 10
+#define CHANGE_MEDIA_RETRIES 20
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
Michal
--
4:51 a.m.
On 18.05.2013 05:31, Li Wei wrote:
On 05/17, Michal Privoznik wrote:
> On 17.05.2013 11:37, Li Wei wrote:
>> On 05/17/2013 04:52 PM, Michal Privoznik wrote:
>>> On 17.05.2013 10:16, Li Wei wrote:
>>>> On 05/17/2013 04:07 PM, Michal Privoznik wrote:
>>>>> On 17.05.2013 07:47, Li Wei wrote:
>>>>>> Hi Michal, Daniel
>>>>>>
>>>>>> It seems there is something wrong with the 2/2 part of this
patchset.
>>>>>> When I do an "change-media" command in virsh, it
doesn't do any better
>>>>>> than before, but even worse(I must to wait 5 secs to see the
error).
>>>>>>
>>>>>> I'm not family with libvirt, just add some log things in the
qemu_hotplug.c
>>>>>> and found the tray_status never change to open, but with
michal's original
>>>>>> patch (which do active poll on tray_status), I can do
"change-media" successfully
>>>>>> every time.
>>>>>
>>>>> Yes & no. Thing is - originally we ignored tray status in the
guest. So
>>>>> in case guest has locked the tray and thus ignored our eject request,
we
>>>>> have changed the media anyway, resulting in possible I/O errors
within
>>>>> guest.
>>>>>
>>>>> With my patch, we send eject request to the qemu, and actively poll
for
>>>>> the tray to open. We check for up to 10 times every 200ms. So guest
has
>>>>> 2 seconds to open the tray. If the guest doesn't eject media and
open
>>>>> the tray we don't proceed with forcibly changing the media.
>>>>>
>>>>> These patches are actually a bug fix for
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>>>>
>>>> Hmm... I think you misunderstood my meaning(sorry for my pool english),
>>>> simply to say, I can get expected behavior by apply patch V1 but not V2.
>>>>
>>>> In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we just
>>>> wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN
but
>>>> this never happened.
>>>>
>>>> Thanks,
>>>> Wei
>>>
>>> Aaah, now I understand. So you are saying that the event is not being
>>> delivered. What's the qemu version?
>>
>>
>> I have the following version:
>>
>> virsh # version
>> Compiled against library: libvirt 1.0.5
>> Using library: libvirt 1.0.5
>> Using API: QEMU 1.0.5
>> Running hypervisor: QEMU 1.4.1
>>
>>>
>>> If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
>>> http://wiki.libvirt.org/page/DebugLogs
>>>
>>> Or maybe the timeout is just short for your guest to eject the tray. So
>>> after a while, does 'virsh change-media' succeed?
>>
>> Yes, I can see the DEVICE_TRAY_MOVED in the log file, it goes after
>> the debug message "qemuDomainChangeEjectableMedia:103 : Waiting 500ms for
tray to open. Retries left 0".
>> does that mean the timeout is too short?
>>
>> If the guest needs too much time to eject, I'm wondering why when I use
>> the v1 patch, I can get succeed in about 1 to 2 seconds. I doubt if there
>> are other problems cause tray_status not updated in time?
>>
>> I appended the log file for your reference, if you need any other information,
>> please tell me.
>>
>> Thanks,
>> Wei
>
> I am pasting some interesting knobs from the log:
>
> 2013-05-17 09:04:46.234+0000: 27004: debug : qemuMonitorSend:892 :
QEMU_MONITOR_SEND_MSG: mon=0x7f7ac4000bd0
msg={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
> fd=-1
> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOWrite:458 :
QEMU_MONITOR_IO_WRITE: mon=0x7f7ac4000bd0
buf={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
> len=92 ret=92 errno=2
> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"id": "libvirt-7",
"error": {"class": "GenericError", "desc":
"Device 'drive-ide0-1-0' is locked"}}
> len=102
> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:46.235+0000: 27004: error : qemuMonitorJSONCheckError:352 : internal
error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is
locked
> 2013-05-17 09:04:46.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 9
> 2013-05-17 09:04:46.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 8
> 2013-05-17 09:04:47.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 7
> 2013-05-17 09:04:47.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 6
> 2013-05-17 09:04:48.186+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:48.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 5
> 2013-05-17 09:04:48.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 4
> 2013-05-17 09:04:49.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 3
> 2013-05-17 09:04:49.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 2
> 2013-05-17 09:04:50.236+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 1
> 2013-05-17 09:04:50.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 0
> 2013-05-17 09:04:51.236+0000: 27004: error : qemuDomainChangeEjectableMedia:117 :
operation failed: Unable to eject media before changing it
> 2013-05-17 09:04:51.236+0000: 27004: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
> 2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockDiskDetach:282 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 disk=0x7f7ac4003690
> 2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockManagerNew:128 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 withResources=0
> 2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerPluginGetDriver:295 :
plugin=0x7f7acc007c00
> 2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerNew:319 :
driver=0x7f7ae53dc5e0 type=0 nparams=5 params=0x7f7ad7d25830 flags=0
> 2013-05-17 09:04:51.236+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"timestamp":
{"seconds": 1368781488, "microseconds": 186399}, "event":
"DEVICE_TRAY_MOVED", "data": {"device":
"drive-ide0-1-0", "tray-open": true}}
> len=151
>
>
> What we can see here is:
> 1) libvirt asks qemu to eject
> 2) qemu responds with "Device is locked" - which is correct as it emulates
guest request
> 3) However, after a while, the guest decides it's now safe to eject the media
(e.g. all reads have finished, and the device has been unmounted) which is done
> 4) [not in the pasted log] After a while, you've decided to try again and this
time it succeeded, as the tray is already opened.
>
> What we may do here is lifting the number of retries up. You can try yourself:
I have indeed increased retries to 20 and unfortunately that didn't work either,
we can see in the log file that the DEVICE_TRAY_MOVED event received immediately
after the final retry. After we doubled the retries, that would be more that enough
time for the guest to eject the tray. So, maybe there is something wrong otherwhere.
Thanks,
Wei
I think I know where the bug is. When waiting on tray to open, we don't unlock the
domain. So later, when an event arrives, the first thing that the event handler does is
locking the domain. Hence, it seems like the event is delivered right after we've
failed waiting. That's just because after we've finished waiting, we've
unlocked the domain. And I think same applies for the domain monitor. So can you please
try this out:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d037c9d..767d5a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -95,14 +95,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+ virObjectRef(vm);
/* we don't want to report errors from media tray_open polling */
while (retries--) {
if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
break;
+ qemuDomainObjExitMonitor(driver, vm);
+ virObjectUnlock(vm);
VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d", retries);
usleep(500 * 1000); /* sleep 500ms */
+ virObjectLock(vm);
+ qemuDomainObjEnterMonitor(driver, vm);
}
+ virObjectUnref(vm);
if (disk->src) {
/* deliberately don't depend on 'ret' as 'eject' may have
failed for the
Michal
7:50 p.m.
On 05/19/2013 05:51 PM, Michal Privoznik wrote:
On 18.05.2013 05:31, Li Wei wrote:
> On 05/17, Michal Privoznik wrote:
>> On 17.05.2013 11:37, Li Wei wrote:
>>> On 05/17/2013 04:52 PM, Michal Privoznik wrote:
>>>> On 17.05.2013 10:16, Li Wei wrote:
>>>>> On 05/17/2013 04:07 PM, Michal Privoznik wrote:
>>>>>> On 17.05.2013 07:47, Li Wei wrote:
>>>>>>> Hi Michal, Daniel
>>>>>>>
>>>>>>> It seems there is something wrong with the 2/2 part of this
patchset.
>>>>>>> When I do an "change-media" command in virsh, it
doesn't do any better
>>>>>>> than before, but even worse(I must to wait 5 secs to see the
error).
>>>>>>>
>>>>>>> I'm not family with libvirt, just add some log things in
the qemu_hotplug.c
>>>>>>> and found the tray_status never change to open, but with
michal's original
>>>>>>> patch (which do active poll on tray_status), I can do
"change-media" successfully
>>>>>>> every time.
>>>>>>
>>>>>> Yes & no. Thing is - originally we ignored tray status in the
guest. So
>>>>>> in case guest has locked the tray and thus ignored our eject
request, we
>>>>>> have changed the media anyway, resulting in possible I/O errors
within
>>>>>> guest.
>>>>>>
>>>>>> With my patch, we send eject request to the qemu, and actively
poll for
>>>>>> the tray to open. We check for up to 10 times every 200ms. So
guest has
>>>>>> 2 seconds to open the tray. If the guest doesn't eject media
and open
>>>>>> the tray we don't proceed with forcibly changing the media.
>>>>>>
>>>>>> These patches are actually a bug fix for
>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=892289
>>>>>
>>>>> Hmm... I think you misunderstood my meaning(sorry for my pool
english),
>>>>> simply to say, I can get expected behavior by apply patch V1 but not
V2.
>>>>>
>>>>> In V1, we do active poll on qemuMonitorGetBlockInfo(), and in V2 we
just
>>>>> wait for origdisk->tray_status to become VIR_DOMAIN_DISK_TRAY_OPEN
but
>>>>> this never happened.
>>>>>
>>>>> Thanks,
>>>>> Wei
>>>>
>>>> Aaah, now I understand. So you are saying that the event is not being
>>>> delivered. What's the qemu version?
>>>
>>>
>>> I have the following version:
>>>
>>> virsh # version
>>> Compiled against library: libvirt 1.0.5
>>> Using library: libvirt 1.0.5
>>> Using API: QEMU 1.0.5
>>> Running hypervisor: QEMU 1.4.1
>>>
>>>>
>>>> If you turn the debug logs on, do you see DEVICE_TRAY_MOVED in them?
>>>> http://wiki.libvirt.org/page/DebugLogs
>>>>
>>>> Or maybe the timeout is just short for your guest to eject the tray. So
>>>> after a while, does 'virsh change-media' succeed?
>>>
>>> Yes, I can see the DEVICE_TRAY_MOVED in the log file, it goes after
>>> the debug message "qemuDomainChangeEjectableMedia:103 : Waiting 500ms
for tray to open. Retries left 0".
>>> does that mean the timeout is too short?
>>>
>>> If the guest needs too much time to eject, I'm wondering why when I use
>>> the v1 patch, I can get succeed in about 1 to 2 seconds. I doubt if there
>>> are other problems cause tray_status not updated in time?
>>>
>>> I appended the log file for your reference, if you need any other
information,
>>> please tell me.
>>>
>>> Thanks,
>>> Wei
>>
>> I am pasting some interesting knobs from the log:
>>
>> 2013-05-17 09:04:46.234+0000: 27004: debug : qemuMonitorSend:892 :
QEMU_MONITOR_SEND_MSG: mon=0x7f7ac4000bd0
msg={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
>> fd=-1
>> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOWrite:458 :
QEMU_MONITOR_IO_WRITE: mon=0x7f7ac4000bd0
buf={"execute":"eject","arguments":{"device":"drive-ide0-1-0","force":false},"id":"libvirt-7"}
>> len=92 ret=92 errno=2
>> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:46.234+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"id": "libvirt-7",
"error": {"class": "GenericError", "desc":
"Device 'drive-ide0-1-0' is locked"}}
>> len=102
>> 2013-05-17 09:04:46.234+0000: 27001: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:46.235+0000: 27004: error : qemuMonitorJSONCheckError:352 :
internal error unable to execute QEMU command 'eject': Device
'drive-ide0-1-0' is locked
>> 2013-05-17 09:04:46.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 9
>> 2013-05-17 09:04:46.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 8
>> 2013-05-17 09:04:47.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 7
>> 2013-05-17 09:04:47.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 6
>> 2013-05-17 09:04:48.186+0000: 27001: debug : virObjectRef:295 : OBJECT_REF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:48.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 5
>> 2013-05-17 09:04:48.735+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 4
>> 2013-05-17 09:04:49.235+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 3
>> 2013-05-17 09:04:49.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 2
>> 2013-05-17 09:04:50.236+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 1
>> 2013-05-17 09:04:50.736+0000: 27004: debug : qemuDomainChangeEjectableMedia:103 :
Waiting 500ms for tray to open. Retries left 0
>> 2013-05-17 09:04:51.236+0000: 27004: error : qemuDomainChangeEjectableMedia:117 :
operation failed: Unable to eject media before changing it
>> 2013-05-17 09:04:51.236+0000: 27004: debug : virObjectUnref:258 : OBJECT_UNREF:
obj=0x7f7ac4000bd0
>> 2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockDiskDetach:282 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 disk=0x7f7ac4003690
>> 2013-05-17 09:04:51.236+0000: 27004: debug : virDomainLockManagerNew:128 :
plugin=0x7f7acc007c00 dom=0x7f7acc0a4160 withResources=0
>> 2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerPluginGetDriver:295 :
plugin=0x7f7acc007c00
>> 2013-05-17 09:04:51.236+0000: 27004: debug : virLockManagerNew:319 :
driver=0x7f7ae53dc5e0 type=0 nparams=5 params=0x7f7ad7d25830 flags=0
>> 2013-05-17 09:04:51.236+0000: 27001: debug : qemuMonitorIOProcess:350 :
QEMU_MONITOR_IO_PROCESS: mon=0x7f7ac4000bd0 buf={"timestamp":
{"seconds": 1368781488, "microseconds": 186399}, "event":
"DEVICE_TRAY_MOVED", "data": {"device":
"drive-ide0-1-0", "tray-open": true}}
>> len=151
>>
>>
>> What we can see here is:
>> 1) libvirt asks qemu to eject
>> 2) qemu responds with "Device is locked" - which is correct as it
emulates guest request
>> 3) However, after a while, the guest decides it's now safe to eject the media
(e.g. all reads have finished, and the device has been unmounted) which is done
>> 4) [not in the pasted log] After a while, you've decided to try again and
this time it succeeded, as the tray is already opened.
>>
>> What we may do here is lifting the number of retries up. You can try yourself:
>
> I have indeed increased retries to 20 and unfortunately that didn't work either,
> we can see in the log file that the DEVICE_TRAY_MOVED event received immediately
> after the final retry. After we doubled the retries, that would be more that enough
> time for the guest to eject the tray. So, maybe there is something wrong otherwhere.
>
> Thanks,
> Wei
I think I know where the bug is. When waiting on tray to open, we don't unlock the
domain. So later, when an event arrives, the first thing that the event handler does is
locking the domain. Hence, it seems like the event is delivered right after we've
failed waiting. That's just because after we've finished waiting, we've
unlocked the domain. And I think same applies for the domain monitor. So can you please
try this out:
This patch works fine, thanks :)
Tested-by: Li Wei <lw(a)cn.fujitsu.com>
Thanks,
Wei
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d037c9d..767d5a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -95,14 +95,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+ virObjectRef(vm);
/* we don't want to report errors from media tray_open polling */
while (retries--) {
if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
break;
+ qemuDomainObjExitMonitor(driver, vm);
+ virObjectUnlock(vm);
VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d",
retries);
usleep(500 * 1000); /* sleep 500ms */
+ virObjectLock(vm);
+ qemuDomainObjEnterMonitor(driver, vm);
}
+ virObjectUnref(vm);
if (disk->src) {
/* deliberately don't depend on 'ret' as 'eject' may have
failed for the
Michal
11:25 a.m.
On 05/19/2013 03:51 AM, Michal Privoznik wrote:
I think I know where the bug is. When waiting on tray to open, we don't unlock the
domain. So later, when an event arrives, the first thing that the event handler does is
locking the domain. Hence, it seems like the event is delivered right after we've
failed waiting. That's just because after we've finished waiting, we've
unlocked the domain. And I think same applies for the domain monitor. So can you please
try this out:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d037c9d..767d5a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -95,14 +95,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
+ virObjectRef(vm);
/* we don't want to report errors from media tray_open polling */
while (retries--) {
if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
break;
+ qemuDomainObjExitMonitor(driver, vm);
+ virObjectUnlock(vm);
VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d",
retries);
usleep(500 * 1000); /* sleep 500ms */
Yep, we absolutely must NOT usleep() while holding locks.
+ virObjectLock(vm);
+ qemuDomainObjEnterMonitor(driver, vm);
}
+ virObjectUnref(vm);
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:26 a.m.
On 20.05.2013 18:25, Eric Blake wrote:
On 05/19/2013 03:51 AM, Michal Privoznik wrote:
>
> I think I know where the bug is. When waiting on tray to open, we don't unlock
the domain. So later, when an event arrives, the first thing that the event handler does
is locking the domain. Hence, it seems like the event is delivered right after we've
failed waiting. That's just because after we've finished waiting, we've
unlocked the domain. And I think same applies for the domain monitor. So can you please
try this out:
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index d037c9d..767d5a1 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -95,14 +95,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> qemuDomainObjEnterMonitor(driver, vm);
> ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
>
> + virObjectRef(vm);
> /* we don't want to report errors from media tray_open polling */
> while (retries--) {
> if (origdisk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)
> break;
>
> + qemuDomainObjExitMonitor(driver, vm);
> + virObjectUnlock(vm);
> VIR_DEBUG("Waiting 500ms for tray to open. Retries left %d",
retries);
> usleep(500 * 1000); /* sleep 500ms */
Yep, we absolutely must NOT usleep() while holding locks.
> + virObjectLock(vm);
> + qemuDomainObjEnterMonitor(driver, vm);
> }
> + virObjectUnref(vm);
ACK
In fact, I am gonna be sending a slightly modified & more readable
version in a little while.
Michal
4203
days inactive
4318
days old
22 comments
6 participants
participants (6)
-
Daniel P. Berrange -
Eric Blake -
Li Wei -
Li Wei -
Michal Privoznik -
Peter Krempa