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

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.