Re: [Xen-devel] [PATCH] Fix cd-insert problem not detecting type.

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Fri, 2013-03-15 at 14:51 +0000, Stefano Stabellini wrote:
> On Tue, 12 Mar 2013, Frediano Ziglio wrote:
> > In 4.1 you could use prefixes that using a regex are
> > 
> > (phy|file|tap2?:(aio:)?(vhd|qcow|qcow2|raw)):
> > 
> > you could use similar syntax in Xml file. If you run just "xl cd-insert"
> > output is
> 
> XML file? Are you sure that you are talking about xl and xm or something
> else?
> 

Was xl configuration files.

> 
> > 'xl cd-insert' requires at least 3 arguments.
> > 
> > Usage: xl [-vfN] cd-insert <Domain> <VirtualDevice> <type:path>
> > 
> > Insert a cdrom into a guest's cd drive.
> > 
> > 
> > so anyway user still expect a similar syntax. block-attach support a
> > different syntax where parameters are split using different command line
> > arguments. Now it seems that code was changed trying to embed old syntax
> > in new parameters way but target parameter was used which does not
> > support old syntax.
> > 
> > It seems that libxlu_disk_l.l try even to emulate the old syntax but it
> > does not set correctly "old" format and backend, for instance for raw
> > should set backend to tap and format to raw while it just set format.
> 
> That is correct because libxl is able to figure out which one is the
> best backend for "raw", that nowadays usually ends up being qemu.
> 
> 
> > > My proposal is that we should bite this bullet and regularise the
> > > parsing, so that xl cd-insert takes the same kind of argument as xl
> > > block-attach.
> 
> I don't know if supporting the full disk-spec-component syntax is the
> best thing to do here: for instance you are not supposed to pass a
> devtype different from cdrom.
> At the very least we need to clarify that devtype is not respected
> when passed to cd-insert.
> 
> 
> > > >   while updating internal code so perhaps the better option is to
> > > > detect is user tried to give a specific type and add the correct
> > > > "format" option to pass to parse_disk_config.  If you agree with
> > > > this I can write a patch. Is not clear however which "types" are
> > > > supported for xl cd-insert command. It seems a mix of backend (like
> > > > "phy") and formats (like "raw"). Is this expected?
> > > 
> > > These "types" are a xend thing and are indeed a mixture of backend,
> > > format, and other information.  In xl they are supported (where they
> > > are) for backward compatibility, but they are not recommended.
> > > 
> > 
> > By the way, do we want to break cd-insert command line syntax or just
> > add a new format?
> > Or is it better to add a new "block-change" command?
> > Currently using 4.2 you can't specify as many format as 4.1.
> 
> I would hate to have to introduce yet another command to work around a
> broken one.
> I would rather break compatibility with the existing cd-insert.

I would be all right with you if we removed supported for this syntax
but as xl configurations files are still supported with this syntax I
prefer to not change cd-insert one.

Frediano

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel