Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to have a proper cleanup that doesn't remove the whole device altogether. Steps a, b, c are the result of my (admittedly quick) audit. Here's how the various objects are connected to each other: contains drivelist--- DriveInfo | | .bdrv | .id == .bdrv-device_name | contains V bdrv_states --- BlockDriverState
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to have a proper cleanup that doesn't remove the whole device altogether. Steps a, b, c are the result of my (admittedly quick) audit. Here's how the various objects are connected to each other: contains drivelist--- DriveInfo
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to have a proper cleanup that doesn't remove the whole device altogether. Steps a, b, c are the result of my (admittedly quick) audit. Here's how the various objects are connected to each other:
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to have a
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Mon, Nov 08, 2010 at 01:03:18PM +0100, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. Sometimes it takes a tough man to make a tender chicken. I'll take my best shot at trying
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Mon, Nov 08, 2010 at 06:56:02PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: Here's how the various objects are connected to each other: contains drivelist--- DriveInfo | | .bdrv | .id == .bdrv-device_name | contains V bdrv_states --- BlockDriverState | ^ .peer | | | | host part -|---|--- | | guest part | | property drive v | DeviceState To disconnect host from guest part, you need to cut both pointers. To delete the host part, you need to delete both objects, BlockDriverState and DriveInfo. If we remove DriveInfo, how can management later detect that guest part was deleted? Directly: check whether the qdev is gone. I don't know how to check that indirectly, via DriveInfo. If you want symmetry with netdev, it's possible to keep a shell of BlockDriverState/DriveInfo around (solving dangling pointer problems). netdev_del deletes the host network part: (qemu) info network Devices not on any VLAN: net.0: net=10.0.2.0, restricted=n peer=nic.0 nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0 (qemu) netdev_del net.0 (qemu) info network Devices not on any VLAN: nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0 It leaves around the VLAN object. Since qdev property points to that, it doesn't dangle. In my opinion, drive_del should make the drive vanish from info block, Yeah; that's the right thing to do here. Let me respin the patch with the name change and the additional work to fix up the pointers and ensure that we don't see the drive in info block. Daniel, I'd like your input here: can you live with device diappearing from info block and parsing qdev tree info to figure out whether device is really gone? We don't use info block for anything. Having to parse the full qdev tree to determine if a single device is gone seems rather tedious. It would be better if query-qdev took an optional argument, which is the name of the device to root the tree at. Then checking whether a device named 'foo' is gone just means running 'query-qdev foo' and seeing if that returns an error about the device not existing, then we know it has gone. No need to parse anything. Being able to query the qdev data for a single device, or sub-tree of devices seems useful in its own right. Regards, 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 :|
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-08 10:57]: On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Without (c), it's not a delete. And (c) without (b) leaves a dangling pointer. (c) without (a) fails an assertion in bdrv_delete(). Aside: (b) should probably be folded into bdrv_detach(). Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Daniel P. Berrange berra...@redhat.com [2010-11-08 11:05]: On Mon, Nov 08, 2010 at 06:56:02PM +0200, Michael S. Tsirkin wrote: On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: Here's how the various objects are connected to each other: contains drivelist--- DriveInfo | | .bdrv | .id == .bdrv-device_name | contains V bdrv_states --- BlockDriverState | ^ .peer | | | | host part -|---|--- | | guest part | | property drive v | DeviceState To disconnect host from guest part, you need to cut both pointers. To delete the host part, you need to delete both objects, BlockDriverState and DriveInfo. If we remove DriveInfo, how can management later detect that guest part was deleted? Directly: check whether the qdev is gone. I don't know how to check that indirectly, via DriveInfo. If you want symmetry with netdev, it's possible to keep a shell of BlockDriverState/DriveInfo around (solving dangling pointer problems). netdev_del deletes the host network part: (qemu) info network Devices not on any VLAN: net.0: net=10.0.2.0, restricted=n peer=nic.0 nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0 (qemu) netdev_del net.0 (qemu) info network Devices not on any VLAN: nic.0: model=virtio-net-pci,macaddr=52:54:00:12:34:56 peer=net.0 It leaves around the VLAN object. Since qdev property points to that, it doesn't dangle. In my opinion, drive_del should make the drive vanish from info block, Yeah; that's the right thing to do here. Let me respin the patch with the name change and the additional work to fix up the pointers and ensure that we don't see the drive in info block. Daniel, I'd like your input here: can you live with device diappearing from info block and parsing qdev tree info to figure out whether device is really gone? We don't use info block for anything. Having to parse the full qdev tree to determine if a single device is gone seems rather tedious. It would be better if query-qdev took an optional argument, which is the name of the device to root the tree at. Then checking whether a device named 'foo' is gone just means running 'query-qdev foo' and seeing if that returns an error about the device not existing, then we know it has gone. No need to parse anything. Being able to query the qdev data for a single device, or sub-tree of devices seems useful in its own right. Since I'm not looking forward to parsing info block (easy) nor parsing all of qdev tree (much harder) I really like the query approach. That makes it easy to put a query in the netdev_del/drive_del commands to skip invoking them if the guest has already responded. Regards, 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 :| -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Mon, Nov 08, 2010 at 12:39:01PM -0600, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-08 10:57]: On Mon, Nov 08, 2010 at 08:02:50AM -0600, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-08 06:04]: Michael S. Tsirkin m...@redhat.com writes: On Mon, Nov 08, 2010 at 11:32:01AM +0100, Markus Armbruster wrote: Daniel, I'd like your input here: can you live with device diappearing from info block and parsing qdev tree info to figure out whether device is really gone? AFAICT, libvirt doesn't look at or use info block at all. I'd rather not have to add info block to libvirt; but currently I can't see how else we can determine if we should call drive_unplug if we do a device_del() and the guest removes it before we call drive_unplug(). What happens is that the guest removes the device and when we call drive_unplug() it fails to find the target device (since it was deleted by the guest). Then we fail the PCiDelDisk and libvirt keeps the device config around even though the guest has finished removing it. This needs drive_unplug to return an explicitly identifiable 'no such device' error code, which libvirt can catch and ignore. Making the call to drive_unplug conditional on a check to query-block/query-qdev is really a bug, because it has an designed in race condition which means you need to check for a 'no such device' error code regardless. So it is better to just blindly call drive_unplug and handle the non-fatal failure conditions every time - this ensures that codepath gets exercised more frequently too :-) Regards, 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 :|
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Fri, Nov 05, 2010 at 05:01:49PM +0100, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: On Fri, Nov 05, 2010 at 02:27:49PM +0100, Markus Armbruster wrote: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. I think it's cleaner not to introduce a concept of a disconnected backend. Backends start disconnected, so the concept already exists. One thing that we must be careful to explicitly disallow, is reconnecting guest to another host backend. The reason being that guest might rely on backend features and changing these would break this. Given that, disconnecting without delete isn't helpful. What about disconnect, hot plug new device, connect? Exactly. I don't think we want to support this. New device might not support all features that old one has. Or it may have more features. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. I don't see how it's more dangerous than disconnecting. If guest can't access the backend it might not exist as far as guest is concerned. If we keep disconnect and delete separate operations, we can make delete fail when still connected. Typo insurance. Do you need anything else from me to make progress? Let's go for 3. Need for 1/2 seems dubious, and it's much harder to support.
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-06 04:19]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part a-c need to be done to match netdev_del symmetry? How hard of a req is this? Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :) drive_unplug() works as-is today; so it does feel very combursome at this point. Other than the name change and agreement on how mgmt should invoke the command, it's been a long ride to get here. I'll take my best shot at trying to clean up the other pointers and objects; though on one of my attempts when I took out the dinfo() object that didn't go so well; going to have to audit who uses dinfo and where and what they check before calling it to have a proper cleanup that doesn't remove the whole device altogether. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. Exactly. a. bdrv_detach() to zap the pointer from bdrv to qdev b. zap the pointer from qdev to bdrv c. drive_uninit() to dispose of the host part Step b could be awkward with (3), because you don't know device details. I guess you have to search device properties for a drive property pointing to bdrv. I like (1) because it puts that loop in the one place where it belongs: qdev core. (3) duplicates it in every HOSTDEV_del. Except for netdev_del, which is special because of VLANs. To avoid step b, you could try to keep the bdrv around in a special zombie state. Still have to free the dinfo, but can't use drive_uninit() for that then. If you think I'm overcomplicating this, feel free to prove me wrong with working code :)
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-05 09:18]: On Fri, Nov 05, 2010 at 02:27:49PM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 16:46]: On Wed, Nov 03, 2010 at 03:59:29PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Fri, Nov 05, 2010 at 02:27:49PM +0100, Markus Armbruster wrote: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 16:46]: On Wed, Nov 03, 2010 at 03:59:29PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 16:46]: On Wed, Nov 03, 2010 at 03:59:29PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Michael S. Tsirkin m...@redhat.com writes: On Fri, Nov 05, 2010 at 02:27:49PM +0100, Markus Armbruster wrote: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. I think it's cleaner not to introduce a concept of a disconnected backend. Backends start disconnected, so the concept already exists. One thing that we must be careful to explicitly disallow, is reconnecting guest to another host backend. The reason being that guest might rely on backend features and changing these would break this. Given that, disconnecting without delete isn't helpful. What about disconnect, hot plug new device, connect? 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. I don't see how it's more dangerous than disconnecting. If guest can't access the backend it might not exist as far as guest is concerned. If we keep disconnect and delete separate operations, we can make delete fail when still connected. Typo insurance. Do you need anything else from me to make progress? Let's go for 3. Need for 1/2 seems dubious, and it's much harder to support.
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. Do you need anything else from me to make progress? I think just an agreement on the approach; shouldn't take more than a few hours to respin the qemu and libvirt side.
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-05 11:11]: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-11-05 08:28]: I'd be fine with any of these: 1. A new command device_disconnet ID (or similar name) to disconnect device ID from any host parts. Nice touch: you don't have to know about the device's host part(s) to disconnect it. But it might be more work than the other two. This is sort of what netdev_del() and drive_unplug() are today; we're just saying sever the connection of this device id. No, I have netdev_del as (3). All three options are sort of the same, just different commands with a common purpose. I'd like to rename drive_unplug() to blockdev_del() and call it done. I was looking at libvirt and the right call to netdev_del is already in-place; I'd just need to re-spin my block patch to call blockdev_del() after invoking device_del() to match what is done for net. Unless I'm missing something, you can't just rename: your unplug does not delete the host part. 2. New commands netdev_disconnect, drive_disconnect (or similar names) to disconnect a host part from a guest device. Like (1), except you have to point to the other end of the connection to cut it. What's the advantage here? We need an additional piece of info (host part) in addition to the device id? That's a disadvantage. Possible advantage: implementation could be slightly easier than (1), because you don't have to find the host parts. 3. A new command drive_del ID similar to existing netdev_del. This is (2) fused with delete. Conceptual wart: you can't disconnect and keep the host part around. Moreover, delete is slightly dangerous, because it renders any guest device still using the host part useless. Hrm, I thought that's what (1) is. No. With (1), the argument is a *device* ID, and we disconnect *all* host parts connected to this device (typically just one). With (3), the argument is a netdev/drive ID, and disconnect *this* host part from the peer device. Well, either (1) or (3); I'd like to rename drive_unplug() to blockdev_del() since they're similar function w.r.t removing access to the host resource. And we can invoke them in the same way from libvirt (after doing guest notification, remove access). I'd call it drive_del for now, to match drive_add. OK, drive_del() and as you mentioned, drive_unplug will take out the block driver, but doesn't remove the dinfo object; that ends up dying when we call the device destructor. I think for symmetry we'll want drive_del to remove the dinfo object as well. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-03 16:46]: On Wed, Nov 03, 2010 at 03:59:29PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Thu, Nov 04, 2010 at 11:45:51AM -0500, Ryan Harper wrote: OK. With netdev_del and drive_unplug commands (not sure if we care to change the names to be similar, maybe blockdev_del) in qemu, we can then implement the following in libvirt: 1) detach-device invocation 2) issue device_del to QEMU 2a) notification is sent) 3) issue netdev_del/blockdev_del as appropriate for the device type 4) update guest XML to indicate device has been removed And a fancier version would look like: 1) detach-device invocation 2) issue device_del to QEMU 2a) notification is sent) 3) set a timeout for guest to respond 4) when timeout expires 4a) check if the pci device has been removed by quering QEMU if it hasn't been removed, issue netdev_del/blockdev_del I think it's easier to check the network device: info network and whatever is appropriate for block 5) update guest XML to indicate device has been removed in both cases, I think we'll also want a patch that validates that the pci slot is available before handing it out again; this will handle the case where the guest doesn't respond to the device removal request; our net/blockdev_del command will break the host/guest association, but we don't want to attempt to attach a device to the same slot. Yes, absolutely. And same for qdev device id. Marcus, do you think we're at a point where the mechanisms for explicitly revoking access to the host resource is consistent between net and block? If so, then I suppose I might have a consmetic patch to fix up the monitor command name to line up with the netdev_del. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting. I was really suggesting that your patch is fine :) Sorry about confusion. I was also saying that from what I hear, the pci express support will at some point need interfaces to - notify guest about device removal/addition - get eject from guest - remove device without talking to guest - add device without talking to guest - suppress device deletion on eject All this can be generic and can work through express configuration mechanisms or through acpi for pci. But this is completely separate from unplugging the host backend, which should be possible at any point. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting. I was really suggesting that your patch is fine :) Sorry about confusion. I don't mean eject; I mean responding to the ACPI event by writing a response to the PCI chipset which QEMU then in turn will invoke the qdev_unplug() path which ultimately kills the device and the Drive and BlockState objects. I was also saying that from what I hear, the pci express support will at some point need interfaces to - notify guest about device removal/addition - get eject from guest - remove device without talking to guest - add device without talking to guest - suppress device deletion on eject All this can be generic and can work through express configuration mechanisms or through acpi for pci. But this is
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting. I was really suggesting that your patch is fine :) Sorry about confusion. I don't mean eject; I mean responding to the ACPI event by writing a response to the PCI chipset which QEMU then in turn will invoke the qdev_unplug() path which ultimately kills the device and the Drive and BlockState objects. I was also saying that from what I hear, the pci express support will at some point need interfaces to - notify guest about device removal/addition - get eject from guest - remove device without talking to guest - add device without talking to guest - suppress device deletion on eject All this can be generic and can work through express
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting. I was really suggesting that your patch is fine :) Sorry about confusion. I don't mean eject; I mean responding to the ACPI event by writing a response to the PCI chipset which QEMU then in turn will invoke the qdev_unplug() path which ultimately kills the device and the Drive and BlockState objects. I was also saying that from what I hear, the pci express support will at some point need interfaces to - notify guest
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting. I was really suggesting that your patch is fine :) Sorry about confusion. I don't mean eject; I mean responding to the ACPI event by writing a response to the PCI chipset which QEMU then in
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. By response you mean eject? No, this is not what I was suggesting.
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Wed, Nov 03, 2010 at 03:59:29PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-03 13:03]: On Wed, Nov 03, 2010 at 12:29:10PM -0500, Ryan Harper wrote: * Markus Armbruster arm...@redhat.com [2010-11-03 11:42]: Ryan Harper ry...@us.ibm.com writes: * Michael S. Tsirkin m...@redhat.com [2010-11-03 02:22]: On Tue, Nov 02, 2010 at 03:23:38PM -0500, Ryan Harper wrote: * Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) Yes. Long term I think we will want a way to do that. unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device This is what drive_unplug does, right? Correct. 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Tue, Nov 02, 2010 at 10:40:32AM +0100, Markus Armbruster wrote: C. FOO_unplug You got a patch for drive_unplug. Need netdev_unplug. By the way, I hate unplug, because it suggests relation to hot unplug. What about disconnect? Any preferences? This implies that both parts stay on, just disconnected. This is really surprise removal. While we are at it, can we handle this generically as removal of devices? -- MST
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-11-02 04:40]:
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 11:11]:
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 09:13]:
[Note cc: Michael]
Ryan Harper ry...@us.ibm.com writes:
If I understand your patch correctly, the difference between your
drive_unplug and my blockdev_del is as follows:
* drive_unplug forcefully severs the connection between the host part of
the block device and its BlockDriverState. A shell of the host part
remains, to be cleaned up later. You need forceful disconnect
operation to be able to revoke access to an image whether the guest
cooperates or not. Fair enough.
* blockdev_del deletes a host part. My current version fails when the
host part is in use. I patterned that after netdev_del, which used to
work that way, until commit 2ffcb18d:
Make netdev_del delete the netdev even when it's in use
To hot-unplug guest and host part of a network device, you do:
device_del NIC-ID
netdev_del NETDEV-ID
For PCI devices, device_del merely tells ACPI to unplug the device.
The device goes away for real only after the guest processed the
ACPI
unplug event.
You have to wait until then (e.g. by polling info pci) before you
can
unplug the netdev. Not good.
Fix by removing the in use check from do_netdev_del(). Deleting a
netdev while it's in use is safe; packets simply get routed to the
bit
bucket.
Isn't this the very same problem that's behind your drive_unplug?
Yes it is.
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del} should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part immediately,
even with an uncooperative guest?
Yes, need to ensure that if the mgmt layer (libvirt) has done what it
believes should have disassociated the host block device from the guest,
we want to ensure that the host block device is no longer accessible
from the guest.
Does this need to be separate from device_del?
no, it doesn't have to be. Honestly, I didn't see a clear way to do
something like unplug early in the device_del because that's all pci
device code which has no knowledge of host block devices; having it
disconnect seemed like a layering violation.
Even if your drive_unplug shouldn't fit in that set, we might want it as
a stop-gap. Depends on how urgent the need for it is. Yet another
special-purpose command to be deprecated later.
The fix is urgent; but I'm willing to spin a couple patches if it helps
get this into better shape.
Can we agree on a common solution for block and net? That's why I cc'ed
Michael.
I didn't see a good way to have block behave the same as net; though I
do agree that it would be good to have this be common, long term.
If we can't make them behave 100% the same, then the next best thing is
to offer a preferred way to do things that works similarly enough to let
users ignore the differences.
Possible preferred ways to revoke access to a host part:
A. device_del
Need to make device_del cut the connection right away instead of when
the guest completes unplug.
device_del changes behavior. Any problems with that?
I don't think so; current mgmt consumers assume a cooperative guest and
don't handle an
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Am 02.11.2010 10:40, schrieb Markus Armbruster: Ryan Harper ry...@us.ibm.com writes: * Markus Armbruster arm...@redhat.com [2010-10-29 11:11]: Ryan Harper ry...@us.ibm.com writes: Regardless of the way we choose, we need to think very clearly on how exactly device models should behave when their host part is missing or a zombie, and how that behavior appears in the guest. For net, making it look exactly like a yanked out network cable would make sense to me. What about block? It seems to me that for block it's like cdrom with no disk, floppy with no media, hard disk that's gone bad. I think we we throw EIO back; it's handled gracefully enough. This is what happens when you do a drive_unplug with my patch; the application using the device gets IO errors. That's expected if a drive were to suddently fail (which is what this looks like). And certainly there is some responsibility at the mgmt console to ensure you're not unplugging a drive that you are currently using. Total drive failure works for me. No media is cute, but it's possible only for drives with removable media. I'd rather have all drives behave the same, whether their media is removable or not. But I think we need some way to eject the medium without destroying the whole device. If you handle it as device broken, you need another command for keeping the device, but ejecting the medium, unplugging the network cable, etc. We have eject for block today, but it needs a drive and not a blockdev (and I'm not even sure it really does what I'm thinking of, once again) Kevin
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-02 08:59]:
On Tue, Nov 02, 2010 at 08:46:22AM -0500, Ryan Harper wrote:
* Markus Armbruster arm...@redhat.com [2010-11-02 04:40]:
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for
all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be
trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side
device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix up
the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del}
should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part immediately,
even with an uncooperative guest?
Yes, need to ensure that if the mgmt layer (libvirt) has done what it
believes should have disassociated the host block device from the guest,
we want to ensure that the host block device is no longer accessible
from the guest.
Does this need to be separate from device_del?
no, it doesn't have to be. Honestly, I didn't see a clear way to do
something like unplug early in the device_del because that's all pci
device code which has no knowledge of host block devices; having it
disconnect seemed like a layering violation.
We invoke the cleanup callback, isn't that enough?
Won't that look a bit strange? on device_del, call the cleanup callback
first;, then notify the guest, if the guest responds, I suppose as long
as the cleanup callback can handle being called a second time that'd
work.
I like the idea of disconnect; if part of the device_del method was to
invoke a disconnect method, we could implement that for block, net, etc;
I'd think we'd want to send the notification, then disconnect.
Struggling with whether it's worth having some reasonable timeout
between notification and disconnect.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 11:11]:
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 09:13]:
[Note cc: Michael]
Ryan Harper ry...@us.ibm.com writes:
If I understand your patch correctly, the difference between your
drive_unplug and my blockdev_del is as follows:
* drive_unplug forcefully severs the connection between the host part of
the block device and its BlockDriverState. A shell of the host part
remains, to be cleaned up later. You need forceful disconnect
operation to be able to revoke access to an image whether the guest
cooperates or not. Fair enough.
* blockdev_del deletes a host part. My current version fails when the
host part is in use. I patterned that after netdev_del, which used to
work that way, until commit 2ffcb18d:
Make netdev_del delete the netdev even when it's in use
To hot-unplug guest and host part of a network device, you do:
device_del NIC-ID
netdev_del NETDEV-ID
For PCI devices, device_del merely tells ACPI to unplug the device.
The device goes away for real only after the guest processed the ACPI
unplug event.
You have to wait until then (e.g. by polling info pci) before you can
unplug the netdev. Not good.
Fix by removing the in use check from do_netdev_del(). Deleting a
netdev while it's in use is safe; packets simply get routed to the bit
bucket.
Isn't this the very same problem that's behind your drive_unplug?
Yes it is.
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del} should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part immediately,
even with an uncooperative guest?
Does this need to be separate from device_del?
Even if your drive_unplug shouldn't fit in that set, we might want it as
a stop-gap. Depends on how urgent the need for it is. Yet another
special-purpose command to be deprecated later.
The fix is urgent; but I'm willing to spin a couple patches if it helps
get this into better shape.
Can we agree on a common solution for block and net? That's why I cc'ed
Michael.
I didn't see a good way to have block behave the same as net; though I
do agree that it would be good to have this be common, long term.
If we can't make them behave 100% the same, then the next best thing is
to offer a preferred way to do things that works similarly enough to let
users ignore the differences.
Possible preferred ways to revoke access to a host part:
A. device_del
Need to make device_del cut the connection right away instead of when
the guest completes unplug.
device_del changes behavior. Any problems with that?
Not an option if we need cut the connection to be separate from
device_del.
B. FOO_del
Got netdev_del.
Need drive_del. If drive is in use, replace it by a special dead
drive without a device name (so the ID becomes available for new
drives), then delete the original.
Wart: drive_del doesn't work reliably after device_del, because the
drive is auto-deleted when the guest completes unplug. Not a problem
for my blockdev_add/blockdev_del work-in-progress, because host parts
created with blockdev_add don't auto-delete.
C. FOO_unplug
You got a patch for drive_unplug.
Need netdev_unplug.
By the way, I hate unplug, because it suggests relation to hot
unplug. What about disconnect?
Any preferences?
Currently,
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Tue, Nov 02, 2010 at 09:22:01AM -0500, Ryan Harper wrote:
* Michael S. Tsirkin m...@redhat.com [2010-11-02 08:59]:
On Tue, Nov 02, 2010 at 08:46:22AM -0500, Ryan Harper wrote:
* Markus Armbruster arm...@redhat.com [2010-11-02 04:40]:
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for
all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be
trouble) is
that in the case where the guest responds to device_del induced
ACPI
removal event; the current qdev code already does the host-side
device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix
up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del}
should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part immediately,
even with an uncooperative guest?
Yes, need to ensure that if the mgmt layer (libvirt) has done what it
believes should have disassociated the host block device from the guest,
we want to ensure that the host block device is no longer accessible
from the guest.
Does this need to be separate from device_del?
no, it doesn't have to be. Honestly, I didn't see a clear way to do
something like unplug early in the device_del because that's all pci
device code which has no knowledge of host block devices; having it
disconnect seemed like a layering violation.
We invoke the cleanup callback, isn't that enough?
Won't that look a bit strange? on device_del, call the cleanup callback
first;, then notify the guest, if the guest responds, I suppose as long
as the cleanup callback can handle being called a second time that'd
work.
Well this is exactly what happens with surpise removal.
If you yank a card out the slot, guest only gets notification
afterwards.
I like the idea of disconnect; if part of the device_del method was to
invoke a disconnect method, we could implement that for block, net, etc;
I'd think we'd want to send the notification, then disconnect.
Struggling with whether it's worth having some reasonable timeout
between notification and disconnect.
The problem with this is that it has no analog in real world.
In real world, you can send some notifications to the guest, and you can
remove the card. Tying them together is what created the problem in the
first place.
Timeouts can be implemented by management, maybe with a nice dialog
being shown to the user.
--
MST
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-02 10:56]:
On Tue, Nov 02, 2010 at 09:22:01AM -0500, Ryan Harper wrote:
* Michael S. Tsirkin m...@redhat.com [2010-11-02 08:59]:
On Tue, Nov 02, 2010 at 08:46:22AM -0500, Ryan Harper wrote:
* Markus Armbruster arm...@redhat.com [2010-11-02 04:40]:
I'd like to have some consistency among net, block and char
device
commands, i.e. a common set of operations that work the same
for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be
trouble) is
that in the case where the guest responds to device_del induced
ACPI
removal event; the current qdev code already does the host-side
device
tear down. Not sure if it is OK to do a blockdev_del()
immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to
fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest
part.
device_del should undo device_add. {block,net,char}dev_{add,del}
should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for
backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part
immediately,
even with an uncooperative guest?
Yes, need to ensure that if the mgmt layer (libvirt) has done what it
believes should have disassociated the host block device from the guest,
we want to ensure that the host block device is no longer accessible
from the guest.
Does this need to be separate from device_del?
no, it doesn't have to be. Honestly, I didn't see a clear way to do
something like unplug early in the device_del because that's all pci
device code which has no knowledge of host block devices; having it
disconnect seemed like a layering violation.
We invoke the cleanup callback, isn't that enough?
Won't that look a bit strange? on device_del, call the cleanup callback
first;, then notify the guest, if the guest responds, I suppose as long
as the cleanup callback can handle being called a second time that'd
work.
Well this is exactly what happens with surpise removal.
If you yank a card out the slot, guest only gets notification
afterwards.
Right, though the card ripper can (in some systems) press the removal
button which would send notification. I think I'm fine with not
bothering to notify; this was mgmt interface driven anyhow so who ever
is doing it should have already ensured they weren't using the device.
I like the idea of disconnect; if part of the device_del method was to
invoke a disconnect method, we could implement that for block, net, etc;
I'd think we'd want to send the notification, then disconnect.
Struggling with whether it's worth having some reasonable timeout
between notification and disconnect.
The problem with this is that it has no analog in real world.
In real world, you can send some notifications to the guest, and you can
remove the card. Tying them together is what created the problem in the
first place.
Timeouts can be implemented by management, maybe with a nice dialog
being shown to the user.
Very true. I'm fine with forcing a disconnect during the removal path
prior to notification. Do we want a new disconnect method at the device
level (pci)? or just use the existing removal callback and call that
during the initial hotremov event?
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Cc yamah...@valinux.co.jp, he is working on hotplug for pci
express.
On Tue, Nov 02, 2010 at 11:53:39AM -0500, Ryan Harper wrote:
* Michael S. Tsirkin m...@redhat.com [2010-11-02 10:56]:
On Tue, Nov 02, 2010 at 09:22:01AM -0500, Ryan Harper wrote:
* Michael S. Tsirkin m...@redhat.com [2010-11-02 08:59]:
On Tue, Nov 02, 2010 at 08:46:22AM -0500, Ryan Harper wrote:
* Markus Armbruster arm...@redhat.com [2010-11-02 04:40]:
I'd like to have some consistency among net, block and char
device
commands, i.e. a common set of operations that work the same
for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be
trouble) is
that in the case where the guest responds to device_del
induced ACPI
removal event; the current qdev code already does the
host-side device
tear down. Not sure if it is OK to do a blockdev_del()
immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See
below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to
fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest
part.
device_del should undo device_add.
{block,net,char}dev_{add,del} should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for
backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and
then a
device_del() That's the urgent situation that needs to be
resolved.
What's the exact problem we need to solve urgently?
Is it provide means to cut the connection to the host part
immediately,
even with an uncooperative guest?
Yes, need to ensure that if the mgmt layer (libvirt) has done what it
believes should have disassociated the host block device from the
guest,
we want to ensure that the host block device is no longer accessible
from the guest.
Does this need to be separate from device_del?
no, it doesn't have to be. Honestly, I didn't see a clear way to do
something like unplug early in the device_del because that's all pci
device code which has no knowledge of host block devices; having it
disconnect seemed like a layering violation.
We invoke the cleanup callback, isn't that enough?
Won't that look a bit strange? on device_del, call the cleanup callback
first;, then notify the guest, if the guest responds, I suppose as long
as the cleanup callback can handle being called a second time that'd
work.
Well this is exactly what happens with surpise removal.
If you yank a card out the slot, guest only gets notification
afterwards.
Right, though the card ripper can (in some systems) press the removal
button which would send notification. I think I'm fine with not
bothering to notify;
I think at least for express the port would notice the event
and notify guest anyway, it just happens after the fact,
by necessity.
this was mgmt interface driven anyhow so who ever
is doing it should have already ensured they weren't using the device.
Right. However, I think two additional new interfaces that
1. just send the notification
2. report event on guest eject
would be a good idea. This might tie in well with pci express work where
there's a standard interface for sending these events and for getting
notified that guest is ready for device to be removed.
Guests also might have ways to lock the card so you can not yank it out,
mechanically. This could translate to a failure to do surpise removal,
management can then decide whether killing the guest makes sense.
I like the idea of disconnect; if part of the device_del method was to
invoke a disconnect method, we could implement that for block, net, etc;
I'd think we'd want to send the notification, then disconnect.
Struggling with whether it's worth having some reasonable timeout
between notification and disconnect.
The problem with this is that it has no analog in real world.
In real world, you can send some notifications to the guest, and you can
remove the card. Tying them together is what created the problem in the
first place.
Timeouts can be implemented by management, maybe with a nice dialog
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device A twist is when guest disabled the device already. Then it would just want to remove the device. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Michael S. Tsirkin m...@redhat.com [2010-11-02 14:18]: On Tue, Nov 02, 2010 at 02:01:08PM -0500, Ryan Harper wrote: I like the idea of disconnect; if part of the device_del method was to invoke a disconnect method, we could implement that for block, net, etc; I'd think we'd want to send the notification, then disconnect. Struggling with whether it's worth having some reasonable timeout between notification and disconnect. The problem with this is that it has no analog in real world. In real world, you can send some notifications to the guest, and you can remove the card. Tying them together is what created the problem in the first place. Timeouts can be implemented by management, maybe with a nice dialog being shown to the user. Very true. I'm fine with forcing a disconnect during the removal path prior to notification. Do we want a new disconnect method at the device level (pci)? or just use the existing removal callback and call that during the initial hotremov event? Not sure what you mean by that, but I don't see a device doing anything differently wrt surprise or ordered removal. So probably the existing callback should do. I don't think we need to talk about disconnect: since we decided we are emulating device removal, let's call it just that. Because current the removal process depends on the guest actually responding. What I'm suggesting is that, in Marcus's term, and what drive_unplug() implements, is to disconnect the host block device from the guest device to prevent any further access to it in the case the guest doesn't respond to the removal request made via ACPI. Very specifically, what we're suggesting instead of the drive_unplug() command so to complete the device removal operation without waiting for the guest to respond; that's what's going to happen if we invoke the response callback; it will appear as if the guest responded whether it did or not. What I was suggesting above was to instead of calling the callback for handing the guest response was to add a device function called disconnect which would remove any association of host resources from guest resources before we notified the guest. Thinking about it again I'm not sure this is useful, but if we're going to remove the device without the guests knowledge, I'm not sure how useful sending the removal requests via ACPI is in the first place. My feeling is that I'd like to have explicit control over the disconnect from host resources separate from the device removal *if* we're going to retain the guest notification. If we don't care to notify the guest, then we can just do device removal without notifying the guest and be done with it. I imagine management would typically want to do this: 1. notify guest 2. wait a bit 3. remove device Yes; but this argues for (1) being a separate command from (3) unless we require (3) to include (1) and (2) in the qemu implementation. Currently we implement: 1. device_del (attempt to remove device) 2. notify guest 3. if guest responds, remove device 4. disconnect host resource from device on destruction With my drive_unplug patch we do: 1. disconnect host resource from device 2. device_del (attempt to remove device) 3. notify guest 4. if guest responds, remove device I think we're suggesting to instead do (if we keep disconnect as part of device_del) 1. device_del (attemp to remove device) 2. notify guest 3. invoke device destruction callback resulting in disconnect host resource from device 4. if guest responds, invoke device destruction path a second time. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
[Note cc: Michael] Ryan Harper ry...@us.ibm.com writes: This patch series decouples the detachment of a block device from the removal of the backing pci-device. Removal of a hotplugged pci device requires the guest to respond before qemu tears down the block device. In some cases, the guest may not respond leaving the guest with continued access to the block device. The new monitor command, drive_unplug, will revoke a guests access to the block device independently of the removal of the pci device. The first patch adds a new drive find method, the second patch implements the monitor command and block layer changes. Changes since v3: - Moved QMP command for drive_unplug() to separate patch Changes since v2: - Added QMP command for drive_unplug() Changes since v1: - CodingStyle fixes - Added qemu_aio_flush() to bdrv_unplug() Signed-off-by: Ryan Harper ry...@us.ibm.com If I understand your patch correctly, the difference between your drive_unplug and my blockdev_del is as follows: * drive_unplug forcefully severs the connection between the host part of the block device and its BlockDriverState. A shell of the host part remains, to be cleaned up later. You need forceful disconnect operation to be able to revoke access to an image whether the guest cooperates or not. Fair enough. * blockdev_del deletes a host part. My current version fails when the host part is in use. I patterned that after netdev_del, which used to work that way, until commit 2ffcb18d: Make netdev_del delete the netdev even when it's in use To hot-unplug guest and host part of a network device, you do: device_del NIC-ID netdev_del NETDEV-ID For PCI devices, device_del merely tells ACPI to unplug the device. The device goes away for real only after the guest processed the ACPI unplug event. You have to wait until then (e.g. by polling info pci) before you can unplug the netdev. Not good. Fix by removing the in use check from do_netdev_del(). Deleting a netdev while it's in use is safe; packets simply get routed to the bit bucket. Isn't this the very same problem that's behind your drive_unplug? I'd like to have some consistency among net, block and char device commands, i.e. a common set of operations that work the same for all of them. Can we agree on such a set? Even if your drive_unplug shouldn't fit in that set, we might want it as a stop-gap. Depends on how urgent the need for it is. Yet another special-purpose command to be deprecated later.
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-10-29 09:13]: [Note cc: Michael] Ryan Harper ry...@us.ibm.com writes: This patch series decouples the detachment of a block device from the removal of the backing pci-device. Removal of a hotplugged pci device requires the guest to respond before qemu tears down the block device. In some cases, the guest may not respond leaving the guest with continued access to the block device. The new monitor command, drive_unplug, will revoke a guests access to the block device independently of the removal of the pci device. The first patch adds a new drive find method, the second patch implements the monitor command and block layer changes. Changes since v3: - Moved QMP command for drive_unplug() to separate patch Changes since v2: - Added QMP command for drive_unplug() Changes since v1: - CodingStyle fixes - Added qemu_aio_flush() to bdrv_unplug() Signed-off-by: Ryan Harper ry...@us.ibm.com If I understand your patch correctly, the difference between your drive_unplug and my blockdev_del is as follows: * drive_unplug forcefully severs the connection between the host part of the block device and its BlockDriverState. A shell of the host part remains, to be cleaned up later. You need forceful disconnect operation to be able to revoke access to an image whether the guest cooperates or not. Fair enough. * blockdev_del deletes a host part. My current version fails when the host part is in use. I patterned that after netdev_del, which used to work that way, until commit 2ffcb18d: Make netdev_del delete the netdev even when it's in use To hot-unplug guest and host part of a network device, you do: device_del NIC-ID netdev_del NETDEV-ID For PCI devices, device_del merely tells ACPI to unplug the device. The device goes away for real only after the guest processed the ACPI unplug event. You have to wait until then (e.g. by polling info pci) before you can unplug the netdev. Not good. Fix by removing the in use check from do_netdev_del(). Deleting a netdev while it's in use is safe; packets simply get routed to the bit bucket. Isn't this the very same problem that's behind your drive_unplug? Yes it is. I'd like to have some consistency among net, block and char device commands, i.e. a common set of operations that work the same for all of them. Can we agree on such a set? Yeah; the current trouble (or at least what I perceive to be trouble) is that in the case where the guest responds to device_del induced ACPI removal event; the current qdev code already does the host-side device tear down. Not sure if it is OK to do a blockdev_del() immediately after the device_del. What happens when we do: device_del ACPI to guest blockdev_del /* removes host-side device */ guest responds to ACPI qdev calls pci device removal code qemu attempts to destroy the associated host-side block That may just work today; and if not, it shouldn't be hard to fix up the code to check for NULLs Even if your drive_unplug shouldn't fit in that set, we might want it as a stop-gap. Depends on how urgent the need for it is. Yet another special-purpose command to be deprecated later. The fix is urgent; but I'm willing to spin a couple patches if it helps get this into better shape. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 09:13]:
[Note cc: Michael]
Ryan Harper ry...@us.ibm.com writes:
This patch series decouples the detachment of a block device from the
removal
of the backing pci-device. Removal of a hotplugged pci device requires the
guest to respond before qemu tears down the block device. In some cases,
the
guest may not respond leaving the guest with continued access to the block
device.
The new monitor command, drive_unplug, will revoke a guests access to the
block device independently of the removal of the pci device.
The first patch adds a new drive find method, the second patch implements
the
monitor command and block layer changes.
Changes since v3:
- Moved QMP command for drive_unplug() to separate patch
Changes since v2:
- Added QMP command for drive_unplug()
Changes since v1:
- CodingStyle fixes
- Added qemu_aio_flush() to bdrv_unplug()
Signed-off-by: Ryan Harper ry...@us.ibm.com
If I understand your patch correctly, the difference between your
drive_unplug and my blockdev_del is as follows:
* drive_unplug forcefully severs the connection between the host part of
the block device and its BlockDriverState. A shell of the host part
remains, to be cleaned up later. You need forceful disconnect
operation to be able to revoke access to an image whether the guest
cooperates or not. Fair enough.
* blockdev_del deletes a host part. My current version fails when the
host part is in use. I patterned that after netdev_del, which used to
work that way, until commit 2ffcb18d:
Make netdev_del delete the netdev even when it's in use
To hot-unplug guest and host part of a network device, you do:
device_del NIC-ID
netdev_del NETDEV-ID
For PCI devices, device_del merely tells ACPI to unplug the device.
The device goes away for real only after the guest processed the ACPI
unplug event.
You have to wait until then (e.g. by polling info pci) before you can
unplug the netdev. Not good.
Fix by removing the in use check from do_netdev_del(). Deleting a
netdev while it's in use is safe; packets simply get routed to the bit
bucket.
Isn't this the very same problem that's behind your drive_unplug?
Yes it is.
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del} should
be similarly paired.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
Even if your drive_unplug shouldn't fit in that set, we might want it as
a stop-gap. Depends on how urgent the need for it is. Yet another
special-purpose command to be deprecated later.
The fix is urgent; but I'm willing to spin a couple patches if it helps
get this into better shape.
Can we agree on a common solution for block and net? That's why I cc'ed
Michael.
Currently, we have two different ways:
* The netdev way: del always succeeds
How can it succeed if the host part is in use?
If all device models are prepared to deal with a missing host part, we
can delete it right away.
Else, we need to replace it with a suitable zombie, which is
auto-deleted when it goes out of use. Such zombies are not be visible
elsewhere, in particular, the ID becomes available immediately.
* The unplug way: del fails while in use, unplug always succeeds
Feels a bit cleaner to me. But changing netdev_del might not be
acceptable.
Either way works for me as an user interface. But I'd rather not have
both.
Next, we need to consider how to integrate this with the automatic
deletion of drives on qdev destruction. That's too late for unplug, we
want that right in device_del. I'd leave the stupid automatic delete
where it is now, in qdev destruction. The C API need unplug and delete
separate for that.
Regardless of
Re: [Qemu-devel] [PATCH 0/3] v4 Decouple block device removal from device removal
* Markus Armbruster arm...@redhat.com [2010-10-29 11:11]:
Ryan Harper ry...@us.ibm.com writes:
* Markus Armbruster arm...@redhat.com [2010-10-29 09:13]:
[Note cc: Michael]
Ryan Harper ry...@us.ibm.com writes:
If I understand your patch correctly, the difference between your
drive_unplug and my blockdev_del is as follows:
* drive_unplug forcefully severs the connection between the host part of
the block device and its BlockDriverState. A shell of the host part
remains, to be cleaned up later. You need forceful disconnect
operation to be able to revoke access to an image whether the guest
cooperates or not. Fair enough.
* blockdev_del deletes a host part. My current version fails when the
host part is in use. I patterned that after netdev_del, which used to
work that way, until commit 2ffcb18d:
Make netdev_del delete the netdev even when it's in use
To hot-unplug guest and host part of a network device, you do:
device_del NIC-ID
netdev_del NETDEV-ID
For PCI devices, device_del merely tells ACPI to unplug the device.
The device goes away for real only after the guest processed the ACPI
unplug event.
You have to wait until then (e.g. by polling info pci) before you can
unplug the netdev. Not good.
Fix by removing the in use check from do_netdev_del(). Deleting a
netdev while it's in use is safe; packets simply get routed to the bit
bucket.
Isn't this the very same problem that's behind your drive_unplug?
Yes it is.
I'd like to have some consistency among net, block and char device
commands, i.e. a common set of operations that work the same for all of
them. Can we agree on such a set?
Yeah; the current trouble (or at least what I perceive to be trouble) is
that in the case where the guest responds to device_del induced ACPI
removal event; the current qdev code already does the host-side device
tear down. Not sure if it is OK to do a blockdev_del() immediately
after the device_del. What happens when we do:
device_del
ACPI to guest
blockdev_del /* removes host-side device */
Fails in my tree, because the blockdev's still in use. See below.
guest responds to ACPI
qdev calls pci device removal code
qemu attempts to destroy the associated host-side block
That may just work today; and if not, it shouldn't be hard to fix up the
code to check for NULLs
I hate the automatic deletion of host part along with the guest part.
device_del should undo device_add. {block,net,char}dev_{add,del} should
be similarly paired.
Agreed.
In my blockdev branch, I keep the automatic delete only for backwards
compatibility: if you create the drive with drive_add, it gets
auto-deleted, but if you use blockdev_add, it stays around.
But what to do about the case where we're doing drive_add and then a
device_del() That's the urgent situation that needs to be resolved.
Even if your drive_unplug shouldn't fit in that set, we might want it as
a stop-gap. Depends on how urgent the need for it is. Yet another
special-purpose command to be deprecated later.
The fix is urgent; but I'm willing to spin a couple patches if it helps
get this into better shape.
Can we agree on a common solution for block and net? That's why I cc'ed
Michael.
I didn't see a good way to have block behave the same as net; though I
do agree that it would be good to have this be common, long term.
Currently, we have two different ways:
* The netdev way: del always succeeds
How can it succeed if the host part is in use?
If all device models are prepared to deal with a missing host part, we
can delete it right away.
Else, we need to replace it with a suitable zombie, which is
auto-deleted when it goes out of use. Such zombies are not be visible
elsewhere, in particular, the ID becomes available immediately.
* The unplug way: del fails while in use, unplug always succeeds
Feels a bit cleaner to me. But changing netdev_del might not be
acceptable.
Either way works for me as an user interface. But I'd rather not have
both.
Next, we need to consider how to integrate this with the automatic
deletion of drives on qdev destruction. That's too late for unplug, we
want that right in device_del. I'd leave the stupid automatic delete
where it is now, in qdev destruction. The C API need unplug and delete
separate for that.
Regardless of the way we choose, we need to think very clearly on how
exactly device models should behave when their host part is missing or a
zombie, and how that behavior appears in the guest.
For net, making it look exactly like a yanked out network cable would
make sense to me.
What about block?
It seems to me that for block it's like cdrom with no disk, floppy with
no media, hard disk that's gone bad. I think we we
