On Fri, May 03, 2013 at 08:52:59AM -0500, Anthony Liguori wrote:
"Daniel P. Berrange" <berrange@redhat.com> writes:
On Thu, May 02, 2013 at 10:40:06AM -0500, Anthony Liguori wrote:
Kevin Wolf <kwolf@redhat.com> writes:
> + > + if (strcmp(type, "ide-cd") == 0) { > + disk_type = DT_CDROM; > + } else if (strcmp(type, "isa-fdc") == 0) { > + disk_type = DT_FLOPPY; > + } else { > + disk_type = DT_NORMAL; > + }
Same thing here, comparing against strings is a hack. Devices should probably have a property that says what kind of device they are.
Ack, this is nasty. I would like to eliminate this. There is a type field in BlockInfo but:
# @type: This field is returned only for compatibility reasons, it should # not be used (always returns 'unknown')
I vaguely remember this happening but I don't remember the specific reason why. I would definitely prefer that we filled out type correctly.
I think Markus was involved in this. Markus or Luiz, do you remember the story here?
The reason is that BlockInfo is about the backend and it simply doesn't know (ever since we introduced if=none, this was buggy, so we just abandoned it at some point). We would have to ask the device, not the block layer.
Yes, this makes sense. We could introduce an interface that all disks implemented that returned information about whether it was a CD-ROM, Floppy, etc.
How does libvirt cope with this today? I presume they do something similar to what this patch is doing in terms of hard coding device names.
Sorry, not really sure what your question is here - how does libvirt cope with what exactly ?
Given a device, how do you figure out if it's a cdrom/floppy/whatever without hard coding a mapping of class name -> device type.
Pretty sure libvirt just has a class name mapping, right?
The only place where we'd ever need todo that is when we reverse engineer a libvirt XML config from a set of QEMU command line args. For that we just look at the if=XXX parameter currently. Our reverse engineering code is currently broken for if=none scenarios, due mostly to our laziness in writing code to parse the corresponding -device arg. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|