Re: [PATCH v2 2/3] hw/ide/piix: Add some documentation to pci_piix3_xen_ide_unplug()

[Anthony PERARD via <qemu-devel@nongnu.org>]

On Fri, May 13, 2022 at 08:09:56PM +0200, Bernhard Beschow wrote:
> The comment is based on commit message
> ae4d2eb273b167dad748ea4249720319240b1ac2 'xen-platform: add missing disk
> unplug option'. Since it seems to describe design decisions and
> limitations that still apply it seems worth having.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 2345fe9e1d..bc1b37512a 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -173,6 +173,17 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
>      }
>  }
>  
> +/*
> + * The Xen HVM unplug protocol [1] specifies a mechanism to allow guests to
> + * request unplug of 'aux' disks (which is stated to mean all IDE disks,
> + * except the primary master).
> + *
> + * NOTE: The semantics of what happens if unplug of all disks and 'aux' disks
> + *       is simultaneously requested is not clear. The implementation assumes
> + *       that an 'all' request overrides an 'aux' request.
> + *
> + * [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc
> + */
>  int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
>  {
>      PCIIDEState *pci_ide;

That comments seems to focus on 'aux', but it also gives some pointer on
what calls the function. So it looks fine.

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,


-- 
Anthony PERARD