On Fri, May 03, 2013 at 08:52:59AM -0500, Anthony Liguori wrote: > "Daniel P. Berrange" <berrange@xxxxxxxxxx> writes: > > > On Thu, May 02, 2013 at 10:40:06AM -0500, Anthony Liguori wrote: > >> Kevin Wolf <kwolf@xxxxxxxxxx> 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list