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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list