On Thu, Nov 11, 2010 at 11:29:36AM +0100, Markus Armbruster wrote: > Alon Levy <al...@redhat.com> writes: > > > On Wed, Nov 10, 2010 at 04:49:38PM +0100, Markus Armbruster wrote: > >> Sorry for coming so late to this thread... > >> > >> Alon Levy <al...@redhat.com> writes: > >> > >> > On Thu, Oct 21, 2010 at 08:13:19AM -0500, Anthony Liguori wrote: > >> >> On 10/21/2010 08:03 AM, Gerd Hoffmann wrote: > >> >> >On 10/21/10 08:36, Alon Levy wrote: > >> >> >>v2->v3 changes: > >> >> >> * add configure parameter > >> >> >> * fix docs > >> >> >> > >> >> >>v2 message: > >> >> >>This patchset uses id like device_del for attaching/detaching usb > >> >> >>devices. The first two patches ready the way: > >> >> >> 1. makes qdev_find_recursive non static and in qdev.h > >> >> >> 2. adds a usb_device_by_id which goes over the usb buses calling > >> >> >> qdev_find_recursive > >> >> >> 3. adds the commands that use usb_device_by_id > >> >> >> > >> >> >>Alon Levy (3): > >> >> >> qdev: make qdev_find_recursive public > >> >> >> usb: add public usb_device_by_id > >> >> >> monitor: add usb_attach and usb_detach (v2) > >> >> >> > >> >> > > >> >> >Acked-by: Gerd Hoffmann <kra...@redhat.com> > >> >> > >> >> Okay, I am still confused about the use-case for this and I don't > >> >> see any further explanation in the commit messages. I've seen > >> >> "debugging" but can you be a bit more specific about which cases > >> >> it's needed for? > >> >> > >> > > >> > I use it for debugging the usb-ccid device. I think it's useful for > >> > any other usb device tests as well. The existing commands are not > >> > good enough to do a remove/insert of a usb device, since deleting > >> > a device also deletes any chardev associated with it, and there is > >> > no monitor command to add a chardev. Also sometimes you don't want > >> > to close the chardev, just have the guest see a removal/reinsert of > >> > the device. > >> [...] > >> > >> Let's see whether I get you: detach removes the device, but doesn't > >> destroy it. The only thing you can do with a detached device is attach > >> it. Detach+attach is basically the same as del+add with the same > >> configuration. Except shortcomings in our command set make it > >> impossible to recreate the configuration sometimes. Correct? > > So the problems with the current commands from my pov: > > - device deletion removes associated chardev > > - no way to do it without removing chardev > > - no way to add chardev later and use it for device add > > The outcome of which is that you can't do a guest wise attach/detach > > from monitor if your device relies on a chardev association. This > > happens with my passthrough ccid device. > > Commands chardev_add, chardev_del look feasible to me. > > I hate device_del destroying chardevs automatically. If it was created > separately, it should be destroyed separately. But any fix needs to be > backwards compatible somehow. How to do that without embarrassingly > ugly warts isn't obvious to me. > > >> Questions: > >> > >> 1. If we add commands so that you can always recreate the configuration, > >> is detach+attach still useful? Why? > > If you make it so you can do a device_del and not remove the chardev, and > > later device_add using the already existing chardev, then that will be > > equivalent for me. > > Would chardev_add suffice, or do you need a way to reuse the existing > chardev? > I'd love chardev_add / chardev_del for testing in general, but they don't work for my use case, because chardev_del closes the socket (in my case). I could of course fix my client to work with reconnect, but it doesn't make this pretty.
> >> 2. Why is this a USB problem, and not a general problem? In other > >> words, why usb_{detach,attach}, and not device_{detach,attach}? > > I guess attach/detach is a don't-free-some-resources del/add. If you > > think there are users for a device_attach/detach and it makes sense > > conceptually (what's a detach/attach for an ide bus? for a pci it's > > pretty clear, for sata, etc.) then you could blow this up to a device > > specific callback or something like that (assuming that's how you > > would implement this). > > For buses that don't support hot plug, such as IDE, detach makes as much > sense as delete: none. > > For buses that do (USB, PCI, SCSI, virtio-serial-bus), detach looks like > the first half of delete to me: shut down, remove from device tree > (second half is destroying the device object). > > Likewise, attach looks like the second have of add: insert into device > tree, start up (first half is creating the device object). > > Pitfall: to make re-attach work, qdev method init() needs to work not > just for newly created objects, but after a qdev exit() as well. This > is a change of contract for these two methods. I wouldn't be surprised > if not all of our device were happy with that. > We could flag which devices can do re-attach. Or you go across the board and add a info->detach, info->attach, split from info->exit, info->init. Not a small amount of work :/ Actually, I think you'd need to do that anyway to get any benefit from the detach/attach commands (apart from not deleting associated chardevs).