Re: [Qemu-devel] [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and 'xen-cdrom'

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 07 December 2018 15:26
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; xen-
> devel@lists.xenproject.org; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Stefano Stabellini <sstabellini@kernel.org>
> Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk' and
> 'xen-cdrom'
> 
> On Fri, Dec 07, 2018 at 02:39:40PM +0000, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > > Sent: 07 December 2018 14:35
> > > To: Paul Durrant <Paul.Durrant@citrix.com>
> > > Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org; xen-
> > > devel@lists.xenproject.org; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> > > <mreitz@redhat.com>; Stefano Stabellini <sstabellini@kernel.org>
> > > Subject: Re: [PATCH v2 03/18] xen: introduce 'xen-block', 'xen-disk'
> and
> > > 'xen-cdrom'
> > >
> > > On Thu, Dec 06, 2018 at 03:08:29PM +0000, Paul Durrant wrote:
> > > > +static char *disk_to_vbd_name(unsigned int disk)
> > > > +{
> > > > +    char *name, *prefix = (disk >= 26) ?
> > > > +        disk_to_vbd_name((disk / 26) - 1) : g_strdup("");
> > > > +
> > > > +    name = g_strdup_printf("%s%c", prefix, 'a' + disk);
> > >
> > > I don't think that works, if disk is 27, we do ('a' + 27) here. It's
> > > probably missing a `disk % 26`.
> >
> > Damn, yes I was not allowing the >2 letters.
> >
> > >
> > > > +    g_free(prefix);
> > > > +
> > > > +    return name;
> > > > +}
> > >
> > > [...]
> > >
> > > > +static unsigned int vbd_name_to_disk(const char *name, const char
> > > **endp)
> > > > +{
> > > > +    unsigned int disk = 0;
> > > > +
> > > > +    while (*name != '\0') {
> > > > +        if (!g_ascii_isalpha(*name) || !g_ascii_islower(*name)) {
> > > > +            break;
> > > > +        }
> > > > +
> > > > +        disk *= 26;
> > > > +        disk += *name++ - 'a';
> > > > +    }
> > > > +    *endp = name;
> > > > +
> > > > +    return disk;
> > > > +}
> > > > +
> > > > +static void xen_block_set_vdev(Object *obj, Visitor *v, const char
> > > *name,
> > > > +                               void *opaque, Error **errp)
> > > > +{
> > >
> > > Setting vdev doesn't work. I've tried to add a disk `xvdaa', and it
> > > result in `xvda', or `d0p0' (in the trace). (Same result with
> `xvdaaa',
> > > and 'xvdba' gives 'xvdaa'/d26p0)
> > >
> >
> > Ok, that's weird. I'll have to figure that out.
> 
> It's probably because 'a' is somtime 0 and sometime is 1.
> 
> 'a' should be 0
> 'aa' should be 26,
> 'aaa' Seems to be 702.
> 
> 'xvda': 0     ->                     0 * 1
> 'xvdz': 25    ->                    25 * 1
> 'xvdaa': 26   ->            1 * 26 + 0 * 1
> 'xvdaaa': 702 -> 1 * 26^2 + 1 * 26 + 0 * 1
> 
> So, it's weird. Have fun fixing the algorithm for that.

Actually not as hard as it seemed at first... just treat 'a' as 1 while accumulating the number and then subtract 1 from the result. I wrote a tool to generate the first N disk names using a simple "overflow 'z' to 'aa'" (i.e. non-arithmetical) rule to check against and it matches all the cases I tried (up to 4 letters).

  Paul

> 
> --
> Anthony PERARD