|
From: | Anthony Liguori |
Subject: | [Qemu-devel] Re: [PATCH] net: delay peer host device delete |
Date: | Mon, 20 Sep 2010 15:50:51 -0500 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100826 Lightning/1.0b1 Thunderbird/3.0.7 |
On 09/20/2010 03:37 PM, Michael S. Tsirkin wrote:
On Mon, Sep 20, 2010 at 03:38:36PM -0500, Anthony Liguori wrote:On 09/20/2010 03:27 PM, Michael S. Tsirkin wrote:On Mon, Sep 20, 2010 at 03:20:59PM -0500, Anthony Liguori wrote:On 09/20/2010 02:44 PM, Michael S. Tsirkin wrote:I think the only workable approach that doesn't involve new commands is to change the semantics of the existing ones. Make netdev_del work regardless of whether the device is still present. You would need to reference count the actual netdev structure and have each device using it unref on delete. You make netdev_del mark the device as deleted and when a device is deleted, any calls into the device effectively become nops. You have to go through most of the cleanup process to ensure that tap device gets closed even before your reference count goes to zero.I think you mean 'does not get closed': we need the fd to get the flags etc.No, I actually meant does get closed. When you do netdev_del, it should result in the fd getting closed. The actual netdev structure then becomes a zombie that's completely useless until the device goes away.Note that it will mostly work unless when it'll crash. Issue is we don't have any documentation so people get the command set by trial and error. So how can we prove it's a user bug and not qemu bug? I guess we should blame ourselves until proven innocent.Here's what I'm now suggesting: device_del -> may or may not unplug a device from a guest when it returns. To figure out if it does, you have to run info qdm.I think it should also always unplug on guest reset.netdev_del -> always destroys a netdev device when it returns. May be called at any point in time. If you destroy a netdev while the device is still using it, all packets go into the bit bucket and the link status is modified to be unplugged.One issue here is that we can't allow a new device with same name to be created until the nic is destroyed.A new netdev device? Why not?Because it won't work: it will try to pair with existing nic device (it is looked up by name) and that will fail.
No, netdev_del should remove the VLANClientState from the non_vlan_clients list.
It's no longer enumerable and it's no longer lookup-able.The only reason it stays around it so that the device doesn't have a reference to a free pointer. The only field that's ever looked at is is_deleted which is used by every function to turn around and implement a nop.
The VLANClientState is a hollow shell of it's former glorious self. The remainder of it's (hopefully short) life is merely so that we can avoid touching every device to teach them about disconnecting backends.
Because the fundamental problem is that device_del is too difficult to use. You're just making netdev_del equally difficult to use. Try your patch with libvirt, don't load acpiphp in the guest, and then play around with virsh device_detach and device_attach. All sorts of badness will ensue as libvirt tries to manage assignment of PCI slot numbers and such. Regards, Anthony LiguoriAt some level, that's right. Is yours better?
device_del is still busted but at least netdev_del behaves the way libvirt expects it to.
I guess the right thing is to wait for libvirt guys to tell us what they prefer.
Yeah, I think some clarity is needed. Regards, Anthony Liguori
[Prev in Thread] | Current Thread | [Next in Thread] |