Re: [PATCH 11/12] hw/xen: automatically assign device index to block devices

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Tue, 2023-10-17 at 12:21 +0200, Kevin Wolf wrote:
> Am 16.10.2023 um 17:19 hat David Woodhouse geschrieben:
> > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > 
> > There's no need to force the user to assign a vdev. We can automatically
> > assign one, starting at xvda and searching until we find the first disk
> > name that's unused.
> > 
> > This means we can now allow '-drive if=xen,file=xxx' to work without an
> > explicit separate -driver argument, just like if=virtio.
> > 
> > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> 
> > @@ -34,6 +34,31 @@ static char *xen_block_get_name(XenDevice *xendev, Error 
> > **errp)
> >      XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev);
> >      XenBlockVdev *vdev = &blockdev->props.vdev;
> >  
> > +    if (blockdev->props.vdev.type == XEN_BLOCK_VDEV_TYPE_INVALID) {
> > +        char name[11];
> > +        int disk = 0;
> > +        unsigned long idx;
> > +
> > +        /* Find an unoccupied device name */
> > +        while (disk < (1 << 20)) {
> 
> I like your optimism that we can handle a million disks. :-)

Heh, yeah. Given that there *is* a limit, setting it lower seemed a bit
arbitrary.

For consoles I picked 100 instead of letting it go all the way to
INT_MAX, and in a patch set soon to be posted I'll do the same for the
xen-net-device as I convert it.

Even with a limit of 100, having that many devices *WITHOUT SPECIFYING
WHICH ONE IS WHICH* seems a bit many!

FWIW I've changed it to check for the existence of the *frontend* nodes
(the ones which are visible to the guest). Currently it checks for the
backend nodes, but those might be in different places.

> I haven't reviewed the Xen part in detail, but the patch looks fine on
> the block layer side.
> 
> Acked-by: Kevin Wolf <kwolf@xxxxxxxxxx>

Thanks.

Attachment: smime.p7s
Description: S/MIME cryptographic signature