[Libvir] [PATCH] Have xenDaemonDetachDevice() remove device configuration
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent. This patch updates the code in xenDaemonDetachDevice() to pass the rm_cfg flag to Xen, so that the change is committed to the domain's configuration file. This change depends on a xen patch that was committed to unstable on March 20: http://lists.xensource.com/archives/html/xen-changelog/2008-03/msg00122.html -Ryan diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -3347,7 +3347,7 @@ xenDaemonDetachDevice(virDomainPtr domai if (virDomainXMLDevID(domain, xml, class, ref, sizeof(ref))) return (-1); return(xend_op(domain->conn, domain->name, "op", "device_destroy", - "type", class, "dev", ref, NULL)); + "type", class, "dev", ref, "force", "0", "rm_cfg", "1", NULL)); }
On Tue, Apr 08, 2008 at 01:06:12PM -0700, Ryan Scott wrote:
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent.
This patch updates the code in xenDaemonDetachDevice() to pass the rm_cfg flag to Xen, so that the change is committed to the domain's configuration file.
This change depends on a xen patch that was committed to unstable on March 20: http://lists.xensource.com/archives/html/xen-changelog/2008-03/msg00122.html
but what happens if the flag is passed down while the xend version doesn't support it ? if this breaks we need to check first that the remote support it (based on a version change at some point), isn't it ? Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Tue, Apr 08, 2008 at 01:06:12PM -0700, Ryan Scott wrote:
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent.
This patch updates the code in xenDaemonDetachDevice() to pass the rm_cfg flag to Xen, so that the change is committed to the domain's configuration file.
This change depends on a xen patch that was committed to unstable on March 20: http://lists.xensource.com/archives/html/xen-changelog/2008-03/msg00122.html
but what happens if the flag is passed down while the xend version doesn't support it ? if this breaks we need to check first that the remote support it (based on a version change at some point), isn't it ?
In my tests, if you used a new libvirt with an old xen (i.e. passing the flag when it wasn't expected), the flag was ignored. I didn't test all configurations, but I don't think a version check is necessary. The problems occurred when you used a new xen with old libvirt. Xen would refuse the call if all the flags were not provided. -Ryan
Daniel
On Tue, Apr 08, 2008 at 01:06:12PM -0700, Ryan Scott wrote:
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent.
Actually I wonder what the expectation is. The QEMU driver only implements a very limited form of the virDomainAttachDevice call (merely allowing you to signal that the CD-ROM has been replaced) and doesn't implement virDomainDetachDevice at all, so there is no example to go on there. Maybe better to document what the behaviour is supposed to be in src/libvirt.c? After all you can effect a permanent change to the device configuration by using virDomainDefineXML. Just my 0.02$ Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Wed, Apr 09, 2008 at 11:52:07AM +0100, Richard W.M. Jones wrote:
On Tue, Apr 08, 2008 at 01:06:12PM -0700, Ryan Scott wrote:
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent.
Actually I wonder what the expectation is.
The QEMU driver only implements a very limited form of the virDomainAttachDevice call (merely allowing you to signal that the CD-ROM has been replaced) and doesn't implement virDomainDetachDevice at all, so there is no example to go on there.
Maybe better to document what the behaviour is supposed to be in src/libvirt.c? After all you can effect a permanent change to the device configuration by using virDomainDefineXML.
Please think of the poor user. Dumping the XML, editing it by hand, and feeding it back in barely a user interface at all. virsh detach-disk is. (And yes, I would expect detach-disk to do exactly that, permanently.) regards john
On Wed, Apr 09, 2008 at 12:59:38PM +0100, John Levon wrote:
On Wed, Apr 09, 2008 at 11:52:07AM +0100, Richard W.M. Jones wrote:
On Tue, Apr 08, 2008 at 01:06:12PM -0700, Ryan Scott wrote:
When a user runs 'virsh detach-disk' on a running domain, the disk is removed, but reappears after the domain is rebooted. This seems odd, as someone who types detach-disk most likely wants the change to be permanent.
Actually I wonder what the expectation is.
The QEMU driver only implements a very limited form of the virDomainAttachDevice call (merely allowing you to signal that the CD-ROM has been replaced) and doesn't implement virDomainDetachDevice at all, so there is no example to go on there.
Maybe better to document what the behaviour is supposed to be in src/libvirt.c? After all you can effect a permanent change to the device configuration by using virDomainDefineXML.
Please think of the poor user. Dumping the XML, editing it by hand, and feeding it back in barely a user interface at all. virsh detach-disk is.
(And yes, I would expect detach-disk to do exactly that, permanently.)
I think what's most confusing to the 'poor user' is the inconsistency of behaviour. For example if you were to run that command while the domain is not running, it would detach the device in the configuration file. The fact that the command behaves differently depending on the current state of the domain is what sounds fundemantally wrong to me from an user perspective. This is also one more example why it makes sense to add flags to new operations because in the end both behaviour have their own logic, for example the diffence between (un)mounting a temporary device used while running a domain (say an application install CD-Rom) versus a more persistent data volume. In the end we may want to separate the logic without adding more entry points and an operation flag would have been ideal for this, unfortunately virDomainAttachDevice/virDomainDetachDevice don't have one, too bad. In that specific case of umounting a device, I think it's reasonable to make the operation permanent, as that's the behaviour when the domain is not running, but it's not an ideal situation. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (4)
-
Daniel Veillard -
John Levon -
Richard W.M. Jones -
Ryan Scott