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

From: Anthony PERARD <anthony.perard@citrix.com>
To: Bernhard Beschow <shentey@gmail.com>
Cc: <qemu-devel@nongnu.org>, <qemu-trivial@nongnu.org>,
	<sstabellini@kernel.org>, <paul@xen.org>,
	<xen-devel@lists.xenproject.org>, John Snow <jsnow@redhat.com>,
	"open list:IDE" <qemu-block@nongnu.org>
Subject: Re: [PATCH v2 2/3] hw/ide/piix: Add some documentation to pci_piix3_xen_ide_unplug()
Date: Thu, 19 May 2022 15:33:01 +0100	[thread overview]
Message-ID: <YoZVHYJfBxiemFrr@perard.uk.xensource.com> (raw)
In-Reply-To: <20220513180957.90514-3-shentey@gmail.com>

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