Re: [RFC] xl disk configuration handling

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Mon, 31 Jan 2011, Kamala Narasimhan wrote:
> 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 probably doesn't, but it is better to limit the scope of the fix at
the point.


> > 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?
> 

Yep, that would be fine.


> > 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).
 
It took us several iterations to get the parsing right, I would like to
keep the state machine and the field parsing as it is, but each case
could have its own function to implement it.  In other words, would you
be OK with calling parse_disk_attrib, parse_disk_vdev_info and
parse_disk_pdev_info from the main switch under DSTATE_PHYSPATH,
DSTATE_VIRTPATH, etc?