All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Libvirt <libvir-list@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [libvirt] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices
Date: Fri, 3 May 2013 14:57:06 +0100	[thread overview]
Message-ID: <20130503135706.GM20766@redhat.com> (raw)
In-Reply-To: <87ppx85fwk.fsf@codemonkey.ws>

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

  reply	other threads:[~2013-05-03 14:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-26 19:43 [Qemu-devel] [PATCH 0/3] gtk: add Devices menu Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 1/3] ide: add drive-id property Anthony Liguori
2013-05-02 10:05   ` Andreas Färber
2013-05-02 12:59     ` Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 2/3] monitor: add notifier list for monitor events Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices Anthony Liguori
2013-05-02  8:49   ` Kevin Wolf
2013-05-02 13:41     ` Anthony Liguori
2013-05-02 13:53       ` Luiz Capitulino
2013-05-02 14:06       ` Kevin Wolf
2013-05-02 15:29         ` Markus Armbruster
2013-05-02 15:40         ` Anthony Liguori
2013-05-03 12:31           ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2013-05-03 13:52             ` Anthony Liguori
2013-05-03 13:57               ` Daniel P. Berrange [this message]
2013-05-02 15:47         ` [Qemu-devel] " Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130503135706.GM20766@redhat.com \
    --to=berrange@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.