Re: [RFC] xl disk configuration handling

[Kamala Narasimhan <kamala.narasimhan@gmail.com>]

> I agree that libxl_disk_phystype expresses both the format and the
> backend type in a single confusing way, so there should be two enums:
> one for the format (libxl_disk_format) and one for the backend type
> (libxl_disk_pdev_type).
I will switch to two enums instead of three.

> However I don't think libxl_disk_impl_type should be exposed at all, it
> should be up to libxl to decide whether AIO should be enabled or not. It
> might be useful to let the user disable the PV interface for a
> particular disk (that is what ioemu stands for), but it is too late for
> 4.1, let's just ignore ioemu for the moment.
Ok.

> The backend types should be BLKBACK, TAPDISK2, and QEMU; TAPDISK refers
> to blktap1 that is not supported by libxl. However libxl uses "tap:" as
> backend string corresponding to TAPDISK2, I understand that might be
> confusing but I wouldn't change it now.
> Also it might be useful to retain the EMPTY format among the various
> libxl_disk_format's, it should reduce the overall amount of changes.
>
EMPTY, an indicator that there is no media in the cd-rom drive didn't really go
with the any of the enums which is why I removed it.  But later when I was
changing code I did find it inconvenient to check for both empty path plus
cdrom, so I will add it to disk format types though I am really not sure if it
belongs there.

> it would be nice if all the renaming was done in a separate patch so
> that the real changes are easier to read
>
I was worried you may not accept a patch with just renaming changes!  I could
separate interface changes (which would include renaming) from parsing and send
them as two separate patches. Would that be ok?

>

Stefano - I did go through your comments on a subset of code here but as I
mentioned in my earlier email, please ignore that code for now as I was going to
modify it anyway.  It was mostly to help understand the places that require
change plus for the code to compile.

> 
> do we really need to change the parsing function that much? I
> understand there are significant changes but this is a total rewrite.
> I am concerned about all the bugs we might find later after the
> release...
>
This is one change I would really like to go with.  Not only does it help with
the changes we needed, it also gets rid of code duplication.  With this change
block-attach can rely on the same parsing code (that is once I submit the
block-attach changes patch).
> 
> I would completely ignore "aio:" here.
> I would also ignore "ioemu:" the same way.
>
This redundant logic in block attach for parsing will be gone and disk parsing
logic will be reused.

Kamala