qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] Re: [PATCH] net: delay peer host device delete
Date: Tue, 21 Sep 2010 09:58:14 +0100	[thread overview]
Message-ID: <20100921085814.GC28304@redhat.com> (raw)
In-Reply-To: <20100920193716.GJ30611@redhat.com>

On Mon, Sep 20, 2010 at 09:37:16PM +0200, Michael S. Tsirkin wrote:
> On Mon, Sep 20, 2010 at 02:22:18PM -0500, Anthony Liguori wrote:
> > On 09/20/2010 01:59 PM, Michael S. Tsirkin wrote:
> > >>You can also initiate the unplug from the OS without the ACPI event
> > >>ever happening.  I suspect that in our current implementation, that
> > >>means that we'll automatically delete the device which may have
> > >>strange effects on management tools.
> > >>
> > >>So it probably makes sense for our interface to present the same
> > >>procedure.  What do you think?
> > >>
> > >>Regards,
> > >>
> > >>Anthony Liguori
> > >We seem to have two discussions here. you speak about how an ideal hot plug
> > >interface will look. This can involve new commands etc.
> > >I speak about fixing existing ones so qemu and/or guest won't crash.
> > 
> > To be fair, existing qemu won't crash if you do:
> > 
> > (qemu) device_del <device>
> > Use info_qtree to notice when device goes away
> > (qemu) netdev_del <backend>
> 
> Asking libvirt to busy loop polling the monitor sounds like a really bad
> situation: note that guest is blocked from doing any io while monitor is
> used, so it may in fact prevent it from making progress. Right?

Clearly we need either an async command completion, or an async
event notification of device_del. No one wants todo polling,
nor does anyone sane want to try to parse the outout of info
qtree :-)

 
> So why can't we let management do netdev_del and have it take effect
> when this becomes possible?

That would be really unpleasant to deal with. netdev_del should
always kill the backend immediately, even if the frontend device 
still exists. If this could cause issues for the frontend, then just
connect it to a no-op backend internally so it gets no further data.
In the context of drive_del, once it returns, libvirt changes the security
labelling, so QEMU is guarenteed not to be able to use the backend
anymore, even if it tries to. We would do the same for netdev_del if
we could.

> > You're trying to come up with a workaround for the fact that libvirt
> > is making bad assumptions.
> 
> BTW, even if it is, I don't think we should be crashing qemu or guest.

Even if the libvirt is making bad assumptions about when a monitor
command can be issued, this is no excuse for QEMU crashing. If the
netdev_del can't safely be performed, it should return an error
via the monitor, not crash QEMU.

Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

  parent reply	other threads:[~2010-09-21  8:58 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-20 16:30 [Qemu-devel] [PATCH] net: delay peer host device delete Michael S. Tsirkin
2010-09-20 16:41 ` Anthony Liguori
2010-09-20 16:47   ` Michael S. Tsirkin
2010-09-20 16:56     ` Anthony Liguori
2010-09-20 17:14       ` Michael S. Tsirkin
2010-09-20 18:14         ` Anthony Liguori
2010-09-20 18:19           ` Anthony Liguori
2010-09-20 18:59             ` [Qemu-devel] " Michael S. Tsirkin
2010-09-20 19:22               ` Anthony Liguori
2010-09-20 19:37                 ` Michael S. Tsirkin
2010-09-20 20:15                   ` Anthony Liguori
2010-09-20 20:15                     ` Michael S. Tsirkin
2010-09-21  8:58                   ` Daniel P. Berrange [this message]
2010-09-21  9:20                     ` Michael S. Tsirkin
2010-09-21 12:47                       ` Anthony Liguori
2010-09-20 18:24           ` Michael S. Tsirkin
2010-09-20 18:39             ` Anthony Liguori
2010-09-20 19:15               ` Michael S. Tsirkin
2010-09-20 19:28                 ` Anthony Liguori
2010-09-20 19:44                   ` Michael S. Tsirkin
2010-09-20 20:20                     ` Anthony Liguori
2010-09-20 20:27                       ` Michael S. Tsirkin
2010-09-20 20:38                         ` Anthony Liguori
2010-09-20 20:37                           ` Michael S. Tsirkin
2010-09-20 20:50                             ` Anthony Liguori
2010-09-21  9:18                               ` Michael S. Tsirkin
2010-09-21 12:42                                 ` 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=20100921085814.GC28304@redhat.com \
    --to=berrange@redhat.com \
    --cc=mst@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).