On Fri, Mar 11, 2011 at 04:05:16PM +0100, Jiri Denemark wrote: > > - /* See if drive_del isn't supported */ > > - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { > > - VIR_ERROR0(_("deleting disk is not supported. " > > - "This may leak data if disk is reassigned")); > > - ret = 1; > > - goto cleanup; > > - } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { > > - /* NB: device not found errors mean the drive was > > - * auto-deleted and we ignore the error */ > > - ret = 0; > > - } else { > > - ret = qemuMonitorJSONCheckError(cmd, reply); > > - } > > + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { > > + VIR_DEBUG0(_("drive_del command not found, trying HMP")); > > + ret = qemuMonitorTextDriveDel(mon, drivestr); > > + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { > > + /* NB: device not found errors mean the drive was > > + * auto-deleted and we ignore the error */ > > + ret = 0; > > + } else { > > + ret = qemuMonitorJSONCheckError(cmd, reply); > > } > > Looks good, although I think we should issue the "deleting disk is not > supported. This may leak data if disk is reassigned" error also in case > human-monitor-command is not supported. But that will need some more work on > the HMP infrastructure since it's not possible to get this from > qemuMonitorText* function that we call. Is an "unknown command" reply the information we want? If yes this patch v2 should do the work. -- Thanks, Hu Tao
>From 02aae096cd49445b1eb1d0a5e76d100fdde31ebd Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@xxxxxxxxxxxxxx> Date: Mon, 14 Mar 2011 10:44:47 +0800 Subject: [PATCH v2] qemu: fallback to HMP drive_add/drive_del fallback to HMP drive_add/drive_del commands if not found in QMP changes: v2: - don't translate debug messages - report an error to user if HMP drive_del command not found v1: - fallback to HMP drive_add/drive_del commands if not found in QMP --- src/qemu/qemu_monitor_json.c | 38 ++++++++++++++++++++++---------------- 1 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d97dc68..bf2b403 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2359,11 +2359,18 @@ int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply) < 0)) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("drive_add command not found, trying HMP"); + ret = qemuMonitorTextAddDrive(mon, drivestr); + goto cleanup; + } + + ret = qemuMonitorJSONCheckError(cmd, reply); +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2384,22 +2391,21 @@ int qemuMonitorJSONDriveDel(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; - if (ret == 0) { - /* See if drive_del isn't supported */ - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("drive_del command not found, trying HMP"); + ret = qemuMonitorTextDriveDel(mon, drivestr); + if (ret == 1) VIR_ERROR0(_("deleting disk is not supported. " "This may leak data if disk is reassigned")); - ret = 1; - goto cleanup; - } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { - /* NB: device not found errors mean the drive was - * auto-deleted and we ignore the error */ - ret = 0; - } else { - ret = qemuMonitorJSONCheckError(cmd, reply); - } + } else if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + /* NB: device not found errors mean the drive was + * auto-deleted and we ignore the error */ + ret = 0; + } else { + ret = qemuMonitorJSONCheckError(cmd, reply); } cleanup: -- 1.7.3.1
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list