On Mon, Mar 14, 2011 at 02:20:39PM +0800, Wen Congyang wrote:
At 03/14/2011 10:50 AM, Hu Tao Write:
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.
Jirka said we should issue it when *human-monitor-command* is not supported, not *drive_del* is not supported in text mode.
got it. Thanks for explaination. How can we tell if a command is not supported? A null reply? Or an "unknown command" reply? -- Thanks, Hu Tao