[libvirt] [PATCH v2] qemu: fallback to HMP drive_add/drive_del

[Hu Tao]

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