All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Fix xm block/network-detach command
@ 2007-08-03 20:48 Jim Fehlig
  2007-08-06 11:52 ` Masaki Kanno
  2007-08-07  0:52 ` Jim Fehlig
  0 siblings, 2 replies; 6+ messages in thread
From: Jim Fehlig @ 2007-08-03 20:48 UTC (permalink / raw)
  To: Masaki Kanno; +Cc: xen-devel, mats

Masaki Kanno wrote:
> Hi,
>
> This patch fixes a few problem of xm block/network-detach command. 
>
> At first, when I tested xm block/network-detach command to a inactive 
> managed domain, I saw the following error messages. 
>
> # xm new /xen/vm1.conf
> Using config file "/xen/vm1.conf".
> # xm list --long vm1
> (domain
>  <snip>
>     (device
>         (vif
>             (mac 00:16:3e:75:96:d8)
>             (uuid ebd29601-41ac-75de-87cd-2ae051fa8719)
>         )
>     )
>     (device
>         (vbd
>             (uuid 7572ae87-5706-217a-3fa5-f68496e147c1)
>             (bootable 1)
>             (driver paravirtualised)
>             (dev hda1)
>             (uname file:/xen/rhel4u2.root.img-vm1)
>             (mode w)
>         )
>     )
> )
> # xm block-detach vm1 hda1
> Error: Device hda1 not connected
> Usage: xm block-detach <Domain> <DevId> [-f|--force]
>
> Destroy a domain's virtual block device.
> # xm network-detach vm1 0
> Error: (22, 'Invalid argument, while reading None/device/vif/0/backend')
> Usage: xm network-detach <Domain> <DevId> [-f|--force]
>
> Destroy a domain's virtual network device.
>
>
> I was able to fix the problem of inactive managed domains by writing 
> a small patch.

This brings up a broader question concerning managed domains. What is
the behavior when changing config of a managed domain?

a)
- persist change to store and modify domain if live

b)
- explicitly change live config
- explicitly change stored config

Some of the xm commands support a). That would be behavior of
block-detach with this patch.

It appears Xen API supports b) given the *_live, plug, and unplug
operations defined in the spec. I have noticed this to be the case for
some resource types, but doubt it is consistent across all resource
types - let alone both Xen API and legacy sxpr interfaces.

IMO, since xen itself now supports managed domains it should be possible
to independently change live and stored config. I should be able to feed
a starving domain but have it revert to default (define by me) settings
when reset. That said I think it will be considerable work, particularly
testing, to ensure this behavior across all resource types and xend
interfaces.

>   However, I found a other problem in xm block/network-
> detach command when I tested xm block/network-detach command more. 
> The other problem is the same as the problem that Mats reported in 
> the following. 
>
> http://lists.xensource.com/archives/html/xen-devel/2007-08/msg00040.html
>   

That patch did not cause any problems with save.

> This patch fixes the other problem as follows. 
>  - To remove device info, it waits for the backend path of the device 
>    to be removed. 
>  - It removes device info from domain info. 
>  - It saves domain info to the config.sxp of the managed domain. 
>   

This changes behavior between block-attach and block-detach. In
block-detach, the device is removed from store and unplugged from domain
if live. For block-attach, the operation fails if domain is inactive. If
domain is active, device is plugged but not persisted to store.

We need to agree on behavior when changing config of managed domains and
then set out on the (perhaps time consuming) path of realizing that
behavior.

Thoughts?
Jim

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] Fix xm block/network-detach command
@ 2007-08-03  5:20 Masaki Kanno
  0 siblings, 0 replies; 6+ messages in thread
From: Masaki Kanno @ 2007-08-03  5:20 UTC (permalink / raw)
  To: xen-devel; +Cc: jfehlig, mats

[-- Attachment #1: Mail message body --]
[-- Type: text/plain, Size: 1735 bytes --]

Hi,

This patch fixes a few problem of xm block/network-detach command. 

At first, when I tested xm block/network-detach command to a inactive 
managed domain, I saw the following error messages. 

# xm new /xen/vm1.conf
Using config file "/xen/vm1.conf".
# xm list --long vm1
(domain
 <snip>
    (device
        (vif
            (mac 00:16:3e:75:96:d8)
            (uuid ebd29601-41ac-75de-87cd-2ae051fa8719)
        )
    )
    (device
        (vbd
            (uuid 7572ae87-5706-217a-3fa5-f68496e147c1)
            (bootable 1)
            (driver paravirtualised)
            (dev hda1)
            (uname file:/xen/rhel4u2.root.img-vm1)
            (mode w)
        )
    )
)
# xm block-detach vm1 hda1
Error: Device hda1 not connected
Usage: xm block-detach <Domain> <DevId> [-f|--force]

Destroy a domain's virtual block device.
# xm network-detach vm1 0
Error: (22, 'Invalid argument, while reading None/device/vif/0/backend')
Usage: xm network-detach <Domain> <DevId> [-f|--force]

Destroy a domain's virtual network device.


I was able to fix the problem of inactive managed domains by writing 
a small patch.  However, I found a other problem in xm block/network-
detach command when I tested xm block/network-detach command more. 
The other problem is the same as the problem that Mats reported in 
the following. 

http://lists.xensource.com/archives/html/xen-devel/2007-08/msg00040.html

This patch fixes the other problem as follows. 
 - To remove device info, it waits for the backend path of the device 
   to be removed. 
 - It removes device info from domain info. 
 - It saves domain info to the config.sxp of the managed domain. 


Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>

Best regards,
 Kan


[-- Attachment #2: xm_block_detach.patch --]
[-- Type: application/octet-stream, Size: 8892 bytes --]

diff -r 88bb0d305308 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Wed Aug 01 15:47:54 2007 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Fri Aug 03 14:08:46 2007 +0900
@@ -36,6 +36,7 @@ import xen.lowlevel.xc
 import xen.lowlevel.xc
 from xen.util import asserts
 from xen.util.blkif import blkdev_uname_to_file, blkdev_uname_to_taptype
+from xen.util.blkif import blkdev_name_to_number
 from xen.util import security
 
 from xen.xend import balloon, sxp, uuid, image, arch, osdep
@@ -558,19 +559,65 @@ class XendDomainInfo:
         for devclass in XendDevices.valid_devices():
             self.getDeviceController(devclass).waitForDevices()
 
-    def destroyDevice(self, deviceClass, devid, force = False):
-        try:
-            dev = int(devid)
-        except ValueError:
-            # devid is not a number but a string containing either device
-            # name (e.g. xvda) or device_type/device_id (e.g. vbd/51728)
-            dev = type(devid) is str and devid.split('/')[-1] or None
-            if dev == None:
-                log.debug("Could not find the device %s", devid)
-                return None
-
-        log.debug("dev = %s", dev)
-        return self.getDeviceController(deviceClass).destroyDevice(dev, force)
+    def destroyDevice(self, deviceClass, devid, force = False, rm_cfg = False):
+        dev = self.convertToDeviceNumber(devid)
+        if dev == None:
+            log.debug("Could not find the device %s", devid)
+            return None
+
+        log.debug("XendDomainInfo.destroyDevice: deviceClass = %s, device = %s(%d)",
+                  deviceClass, devid, dev)
+
+        if rm_cfg:
+            # Save current sxprs.  A device number and a backend 
+            # path are needed to remove its configuration but sxprs 
+            # do not have those after calling destroyDevice.
+            sxprs = self.getDeviceSxprs(deviceClass)
+
+        rc = None
+        if self.domid is not None:
+            rc = self.getDeviceController(deviceClass).destroyDevice(dev, force)
+            if not force and rm_cfg:
+                # The backend path, other than the device itself, 
+                # has to be passed because its accompanied frontend 
+                # path may be void until its removal is actually 
+                # issued.  It is probable because destroyDevice is
+                # issued first.
+                for dev_num, dev_info in sxprs:
+                    dev_num = int(dev_num)
+                    if dev_num == dev:
+                        for x in dev_info:
+                            if x[0] == 'backend':
+                                backend = x[1]
+                                break
+                        break
+                self._waitForDevice_destroy(deviceClass, dev, backend)
+
+        if rm_cfg:
+            if deviceClass == 'vif':
+                if self.domid is not None:
+                    for dev_num, dev_info in sxprs:
+                        dev_num = int(dev_num)
+                        if dev_num == dev:
+                            for x in dev_info:
+                                if x[0] == 'mac':
+                                    mac = x[1]
+                                    break
+                            break
+                    dev_info = self.getDeviceInfo_vif(mac)
+                else:
+                    _, dev_info = sxprs[dev]
+            else:  # 'vbd' or 'tap'
+                dev_info = self.getDeviceInfo_vbd(dev)
+            if dev_info is None:
+                return rc
+
+            dev_uuid = sxp.child_value(dev_info, 'uuid')
+            del self.info['devices'][dev_uuid]
+            self.info['%s_refs' % deviceClass].remove(dev_uuid)
+            xen.xend.XendDomain.instance().managed_config_save(self)
+
+        return rc
 
     def getDeviceSxprs(self, deviceClass):
         if self._stateGet() in (DOM_STATE_RUNNING, DOM_STATE_PAUSED):
@@ -1331,6 +1378,10 @@ class XendDomainInfo:
         deviceClass, config = self.info['devices'].get(dev_uuid)
         self._waitForDevice(deviceClass, config['devid'])
 
+    def _waitForDevice_destroy(self, deviceClass, devid, backpath):
+        return self.getDeviceController(deviceClass).waitForDevice_destroy(
+            devid, backpath)
+
     def _reconfigureDevice(self, deviceClass, devid, devconfig):
         return self.getDeviceController(deviceClass).reconfigureDevice(
             devid, devconfig)
@@ -1390,6 +1441,39 @@ class XendDomainInfo:
             self._deviceControllers[name] = devController
     
         return self._deviceControllers[name]
+
+    def convertToDeviceNumber(self, devid):
+        try:
+            dev = int(devid)
+        except ValueError:
+            # devid is not a number but a string containing either
+            # device name (e.g. xvda or xvda:disk) or
+            # device_type/device_id (e.g. vbd/51728)
+            dev = type(devid) is str and devid.split('/')[-1] or None
+            if dev == None:
+                return None
+            try:
+                dev = int(dev)
+            except ValueError:
+                dev = dev.split(':')[0]
+                dev = blkdev_name_to_number(dev)
+        return dev
+
+    def getDeviceInfo_vif(self, mac):
+        for dev_type, dev_info in self.info.all_devices_sxpr():
+            if dev_type != 'vif':
+                continue
+            if mac == sxp.child_value(dev_info, 'mac'):
+                return dev_info
+
+    def getDeviceInfo_vbd(self, devid):
+        for dev_type, dev_info in self.info.all_devices_sxpr():
+            if dev_type != 'vbd' and dev_type != 'tap':
+                continue
+            dev = sxp.child_value(dev_info, 'dev')
+            dev = self.convertToDeviceNumber(dev)
+            if devid == dev:
+                return dev_info
 
     #
     # Migration functions (public)
diff -r 88bb0d305308 tools/python/xen/xend/server/DevController.py
--- a/tools/python/xen/xend/server/DevController.py	Wed Aug 01 15:47:54 2007 +0100
+++ b/tools/python/xen/xend/server/DevController.py	Thu Aug 02 08:46:56 2007 +0900
@@ -28,17 +28,19 @@ from xen.xend.xenstore.xswatch import xs
 
 import os
 
-DEVICE_CREATE_TIMEOUT = 100
+DEVICE_CREATE_TIMEOUT  = 100
+DEVICE_DESTROY_TIMEOUT = 100
 HOTPLUG_STATUS_NODE = "hotplug-status"
 HOTPLUG_ERROR_NODE  = "hotplug-error"
 HOTPLUG_STATUS_ERROR = "error"
 HOTPLUG_STATUS_BUSY  = "busy"
 
-Connected = 1
-Error     = 2
-Missing   = 3
-Timeout   = 4
-Busy      = 5
+Connected    = 1
+Error        = 2
+Missing      = 3
+Timeout      = 4
+Busy         = 5
+Disconnected = 6
 
 xenbusState = {
     'Unknown'      : 0,
@@ -184,6 +186,18 @@ class DevController:
             raise VmError("Device %s (%s) could not be connected.\n%s" %
                           (devid, self.deviceClass, err))
 
+
+    def waitForDevice_destroy(self, devid, backpath):
+        log.debug("Waiting for %s - destroyDevice.", devid)
+
+        if not self.hotplug:
+            return
+
+        status = self.waitForBackend_destroy(backpath)
+
+        if status == Timeout:
+            raise VmError("Device %s (%s) could not be disconnected. " %
+                          (devid, self.deviceClass))
 
 
     def reconfigureDevice(self, devid, config):
@@ -508,6 +522,19 @@ class DevController:
             return (Missing, None)
 
 
+    def waitForBackend_destroy(self, backpath):
+
+        statusPath = backpath + '/' + HOTPLUG_STATUS_NODE
+        ev = Event()
+        result = { 'status': Timeout }
+
+        xswatch(statusPath, deviceDestroyCallback, ev, result)
+
+        ev.wait(DEVICE_DESTROY_TIMEOUT)
+
+        return result['status']
+
+
     def backendPath(self, backdom, devid):
         """Construct backend path given the backend domain and device id.
 
@@ -556,3 +583,19 @@ def hotplugStatusCallback(statusPath, ev
 
     ev.set()
     return 0
+
+
+def deviceDestroyCallback(statusPath, ev, result):
+    log.debug("deviceDestroyCallback %s.", statusPath)
+
+    status = xstransact.Read(statusPath)
+
+    if status is None:
+        result['status'] = Disconnected
+    else:
+        return 1
+
+    log.debug("deviceDestroyCallback %d.", result['status'])
+
+    ev.set()
+    return 0
diff -r 88bb0d305308 tools/python/xen/xm/main.py
--- a/tools/python/xen/xm/main.py	Wed Aug 01 15:47:54 2007 +0100
+++ b/tools/python/xen/xm/main.py	Tue Jul 31 09:28:11 2007 +0900
@@ -2185,6 +2185,7 @@ def xm_network_attach(args):
 
 
 def detach(args, deviceClass):
+    rm_cfg = True
     dom = args[0]
     dev = args[1]
     try:
@@ -2195,7 +2196,7 @@ def detach(args, deviceClass):
     except IndexError:
         force = None
 
-    server.xend.domain.destroyDevice(dom, deviceClass, dev, force)
+    server.xend.domain.destroyDevice(dom, deviceClass, dev, force, rm_cfg)
 
 
 def xm_block_detach(args):

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-08-07 10:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-03 20:48 [PATCH] Fix xm block/network-detach command Jim Fehlig
2007-08-06 11:52 ` Masaki Kanno
2007-08-07  0:52 ` Jim Fehlig
2007-08-07 10:38   ` Masaki Kanno
2007-08-07 10:40     ` Keir Fraser
  -- strict thread matches above, loose matches on Subject: below --
2007-08-03  5:20 Masaki Kanno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.