[libvirt] [PATCH v2] qemu: call drive_unplug in DetachPciDiskDevice

[Daniel P. Berrange]

On Thu, Oct 21, 2010 at 09:56:48PM -0500, Ryan Harper wrote:
> Currently libvirt doesn't confirm whether the guest has responded to the
> disk removal request.  In some cases this can leave the guest with
> continued access to the device while the mgmt layer believes that it has
> been removed.  With a recent qemu monitor command[1] we can
> deterministically revoke a guests access to the disk (on the QEMU side)
> to ensure no futher access is permitted.
> 
> This patch adds support for the drive_unplug() command and introduces it
> in the disk removal paths.  There is some discussion to be had about how
> to handle the case where the guest is running in a QEMU without this
> command (and the fact that we currently don't have a way of detecting
> what monitor commands are available).
> 
> Changes since v1:
>  - return > 0 when command isn't present, < 0 on command failure
>  - detect when drive_unplug command isn't present and log error
>    instead of failing entire command
> 
> Signed-off-by: Ryan Harper <ryanh at us.ibm.com>
> +int qemuMonitorJSONDriveUnplug(qemuMonitorPtr mon,
> +                             const char *drivestr)
> +{

> +
> +    if (ret == 0) {
> +        /* See if drive_unplug isn't supported */
> +        if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> +            qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                            _("unplugging disk is not supported.  "
> +                              "This may leak data if disk is reassigned"));
> +            ret = 1;
> +            goto cleanup;
> +        }
> +        ret = qemuMonitorJSONCheckError(cmd, reply);
> +    }

>  
> +/* Attempts to unplug a drive.  Returns 1 if unsupported, 0 if ok, and -1 on
> + * other failure */
> +int qemuMonitorTextDriveUnplug(qemuMonitorPtr mon,
> +                             const char *drivestr)
> +{
> +


> +    if (strstr(reply, "unknown command:")) {
> +        qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                        _("unplugging disk is not supported.  "
> +                          "This may leak data if disk is reassigned"));
> +        ret = 1;
> +        goto cleanup;

For these 2 non-fatal errors, qemuReportError shouldn't be used. Instead
just directly call VIR_WARN or VIR_ERROR  logging functions

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|