xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError
@ 2013-05-24 17:53 Philipp Hahn
  2013-05-28 15:25 ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Hahn @ 2013-05-24 17:53 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]

Hello,

I noticed a bug in Xen-4.1-3, which is also still present in xen+git.
I know that the Python xend is deprecated, but I'm stuck with xen-4.1 until xen is usable with libvirt, so my patch might still be helpful for others.
This is a follow-up to <http://lists.xen.org/archives/html/xen-users/2012-11/msg00069.html>, which still keeps me busy.

/xen/xend/server/SrvDomain.py declares, that "dev" is a string:
    def op_device_configure(self, _, req):
        return self.call(self.dom.device_configure,
                         [['config', 'sxpr'],
                          ['dev',    'str']],
                         req)

but in xen/xend/XendDomainInfo.py "devid" is expected to be an integer:
    def device_configure(self, dev_sxp, devid = None):
        """Configure an existing device.
...
        @param devid:      device id
        @type  devid:      int

This leads to an error when I try to change the media of an CDROM device:
ERROR (SrvBase:88) Request device_configure failed.
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/xen/web/SrvBase.py", line 85, in perform
    return op_method(op, req)
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/SrvDomain.py", line 216, in op_device_configure
    req)
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/SrvDomain.py", line 186, in call
    return FormFn(fn, args)(req.args)
  File "/usr/lib/python2.6/dist-packages/xen/xend/Args.py", line 166, in __call__
    return self.call_with_form_args(self.fn, fargs, xargs=xargs)
  File "/usr/lib/python2.6/dist-packages/xen/xend/Args.py", line 138, in call_with_form_args
    return fn(*params, **keys)
  File "/usr/lib/python2.6/dist-packages/xen/xend/XendDomainInfo.py", line 1214, in device_configure
    raise VmError("Device %s not connected" % devid)
VmError: Device 768 not connected

This is because devid="768" != dev=768:
    def _getDeviceInfo_vbd(self, devid):
        for dev_type, dev_info in self.info.all_devices_sxpr():
            if dev_type != 'vbd' and dev_type != 'tap' and dev_type != 'tap2':
                continue
            dev = sxp.child_value(dev_info, 'dev')
            dev = dev.split(':')[0]
            dev = self.getDeviceController(dev_type).convertToDeviceNumber(dev)
            if devid == dev:
                return dev_info

After applying the attached patch, I can successfully change the media using libvirt:
# virsh change-media ucs31-64-hvm hda --eject --live --config
# virsh change-media ucs31-64-hvm hda /var/lib/libvirt/images/UCS_3.1-1-amd64.iso --live --config

Sincerely
Philipp Hahn
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        be open.                       fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/

[-- Attachment #2: 23394_xen-change-media.diff --]
[-- Type: text/x-patch, Size: 2450 bytes --]

diff --git a/tools/python/xen/xend/XendDomainInfo.py b/tools/python/xen/xend/XendDomainInfo.py
index e9d3e7e..36dc599 100644
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -1167,7 +1167,7 @@ class XendDomainInfo:
         @param dev_config: device configuration
         @type  dev_config: SXP object (parsed config)
         @param devid:      device id
-        @type  devid:      int
+        @type  devid:      str
         @return: Returns True if successfully updated device
         @rtype: boolean
         """
@@ -1193,24 +1193,24 @@ class XendDomainInfo:
 
         dev_control = self.getDeviceController(dev_class)
         if devid is None:
-            dev = dev_config.get('dev', '')
-            if not dev:
+            devid = dev_config.get('dev', '')
+            if not devid:
                 raise VmError('Block device must have virtual details specified')
-            if 'ioemu:' in dev:
-                (_, dev) = dev.split(':', 1)
+            if 'ioemu:' in devid:
+                (_, devid) = devid.split(':', 1)
             try:
-                (dev, _) = dev.split(':', 1)  # Remove ":disk" or ":cdrom"
+                (devid, _) = devid.split(':', 1)  # Remove ":disk" or ":cdrom"
             except ValueError:
                 pass
-            devid = dev_control.convertToDeviceNumber(dev)
-        dev_info = self._getDeviceInfo_vbd(devid)
+        dev = dev_control.convertToDeviceNumber(devid)
+        dev_info = self._getDeviceInfo_vbd(dev)
         if dev_info is None:
             raise VmError("Device %s not connected" % devid)
         dev_uuid = sxp.child_value(dev_info, 'uuid')
 
         if self.domid is not None:
             # use DevController.reconfigureDevice to change device config
-            dev_control.reconfigureDevice(devid, dev_config)
+            dev_control.reconfigureDevice(dev, dev_config)
         else:
             (_, new_b, new_f) = dev_control.getDeviceDetails(dev_config)
             if (new_f['device-type'] == 'cdrom' and
@@ -1220,7 +1220,7 @@ class XendDomainInfo:
                 pass
             else:
                 raise VmError('Refusing to reconfigure device %s:%d to %s' %
-                              (dev_class, devid, dev_config))
+                              (dev_class, dev, dev_config))
 
         # update XendConfig with new device info
         self.info.device_update(dev_uuid, dev_sxp)

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

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError
  2013-05-24 17:53 [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError Philipp Hahn
@ 2013-05-28 15:25 ` Konrad Rzeszutek Wilk
  2013-05-29  5:47   ` Philipp Hahn
  0 siblings, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-28 15:25 UTC (permalink / raw)
  To: Philipp Hahn; +Cc: xen-devel

On Fri, May 24, 2013 at 07:53:10PM +0200, Philipp Hahn wrote:
> Hello,
> 
> I noticed a bug in Xen-4.1-3, which is also still present in xen+git.
> I know that the Python xend is deprecated, but I'm stuck with xen-4.1 until xen is usable with libvirt, so my patch might still be helpful for others.
> This is a follow-up to <http://lists.xen.org/archives/html/xen-users/2012-11/msg00069.html>, which still keeps me busy.
> 
> /xen/xend/server/SrvDomain.py declares, that "dev" is a string:
>     def op_device_configure(self, _, req):
>         return self.call(self.dom.device_configure,
>                          [['config', 'sxpr'],
>                           ['dev',    'str']],
>                          req)
> 
> but in xen/xend/XendDomainInfo.py "devid" is expected to be an integer:
>     def device_configure(self, dev_sxp, devid = None):
>         """Configure an existing device.
> ...
>         @param devid:      device id
>         @type  devid:      int
> 
> This leads to an error when I try to change the media of an CDROM device:
> ERROR (SrvBase:88) Request device_configure failed.
> Traceback (most recent call last):
>   File "/usr/lib/python2.6/dist-packages/xen/web/SrvBase.py", line 85, in perform
>     return op_method(op, req)
>   File "/usr/lib/python2.6/dist-packages/xen/xend/server/SrvDomain.py", line 216, in op_device_configure
>     req)
>   File "/usr/lib/python2.6/dist-packages/xen/xend/server/SrvDomain.py", line 186, in call
>     return FormFn(fn, args)(req.args)
>   File "/usr/lib/python2.6/dist-packages/xen/xend/Args.py", line 166, in __call__
>     return self.call_with_form_args(self.fn, fargs, xargs=xargs)
>   File "/usr/lib/python2.6/dist-packages/xen/xend/Args.py", line 138, in call_with_form_args
>     return fn(*params, **keys)
>   File "/usr/lib/python2.6/dist-packages/xen/xend/XendDomainInfo.py", line 1214, in device_configure
>     raise VmError("Device %s not connected" % devid)
> VmError: Device 768 not connected
> 
> This is because devid="768" != dev=768:
>     def _getDeviceInfo_vbd(self, devid):
>         for dev_type, dev_info in self.info.all_devices_sxpr():
>             if dev_type != 'vbd' and dev_type != 'tap' and dev_type != 'tap2':
>                 continue
>             dev = sxp.child_value(dev_info, 'dev')
>             dev = dev.split(':')[0]
>             dev = self.getDeviceController(dev_type).convertToDeviceNumber(dev)
>             if devid == dev:
>                 return dev_info

Ewwww, that looks buggy. Could you just do 'return dev' ?

> 
> After applying the attached patch, I can successfully change the media using libvirt:
> # virsh change-media ucs31-64-hvm hda --eject --live --config
> # virsh change-media ucs31-64-hvm hda /var/lib/libvirt/images/UCS_3.1-1-amd64.iso --live --config
> 
> Sincerely
> Philipp Hahn
> -- 
> Philipp Hahn           Open Source Software Engineer      hahn@univention.de
> Univention GmbH        be open.                       fon: +49 421 22 232- 0
> Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
>                                                    http://www.univention.de/

> diff --git a/tools/python/xen/xend/XendDomainInfo.py b/tools/python/xen/xend/XendDomainInfo.py
> index e9d3e7e..36dc599 100644
> --- a/tools/python/xen/xend/XendDomainInfo.py
> +++ b/tools/python/xen/xend/XendDomainInfo.py
> @@ -1167,7 +1167,7 @@ class XendDomainInfo:
>          @param dev_config: device configuration
>          @type  dev_config: SXP object (parsed config)
>          @param devid:      device id
> -        @type  devid:      int
> +        @type  devid:      str
>          @return: Returns True if successfully updated device
>          @rtype: boolean
>          """
> @@ -1193,24 +1193,24 @@ class XendDomainInfo:
>  
>          dev_control = self.getDeviceController(dev_class)
>          if devid is None:
> -            dev = dev_config.get('dev', '')
> -            if not dev:
> +            devid = dev_config.get('dev', '')
> +            if not devid:
>                  raise VmError('Block device must have virtual details specified')
> -            if 'ioemu:' in dev:
> -                (_, dev) = dev.split(':', 1)
> +            if 'ioemu:' in devid:
> +                (_, devid) = devid.split(':', 1)
>              try:
> -                (dev, _) = dev.split(':', 1)  # Remove ":disk" or ":cdrom"
> +                (devid, _) = devid.split(':', 1)  # Remove ":disk" or ":cdrom"
>              except ValueError:
>                  pass
> -            devid = dev_control.convertToDeviceNumber(dev)
> -        dev_info = self._getDeviceInfo_vbd(devid)
> +        dev = dev_control.convertToDeviceNumber(devid)
> +        dev_info = self._getDeviceInfo_vbd(dev)
>          if dev_info is None:
>              raise VmError("Device %s not connected" % devid)
>          dev_uuid = sxp.child_value(dev_info, 'uuid')
>  
>          if self.domid is not None:
>              # use DevController.reconfigureDevice to change device config
> -            dev_control.reconfigureDevice(devid, dev_config)
> +            dev_control.reconfigureDevice(dev, dev_config)
>          else:
>              (_, new_b, new_f) = dev_control.getDeviceDetails(dev_config)
>              if (new_f['device-type'] == 'cdrom' and
> @@ -1220,7 +1220,7 @@ class XendDomainInfo:
>                  pass
>              else:
>                  raise VmError('Refusing to reconfigure device %s:%d to %s' %
> -                              (dev_class, devid, dev_config))
> +                              (dev_class, dev, dev_config))
>  
>          # update XendConfig with new device info
>          self.info.device_update(dev_uuid, dev_sxp)

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError
  2013-05-28 15:25 ` Konrad Rzeszutek Wilk
@ 2013-05-29  5:47   ` Philipp Hahn
  2013-06-05 18:52     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Hahn @ 2013-05-29  5:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel

Hello,

Am Dienstag 28 Mai 2013, 17:25:52 schrieben Sie:
> >     def _getDeviceInfo_vbd(self, devid):
> >         for dev_type, dev_info in self.info.all_devices_sxpr()
...
> >                 return dev_info
> 
> Ewwww, that looks buggy. Could you just do 'return dev' ?

I don't think that this would be correct, since dev and devid are just the device numbers, but dev_info contains the device details  as an Sxpr:
devid='768'
dev='hda:cdrom' ... dev='hda' ... dev=768
dev_type='vbd'
dev_info=['vbd', ('uuid', '920a5c24-28c6-6abb-ad47-9c1144012986'), ('bootable', 1), ('driver', 'paravirtualised'), ('dev', 'hda:cdrom'), ('uname', 'file:/var/lib/libvirt/images/UCS_3.1-1-amd64.iso'), ('mode', 'r'), ('VDI', ''), ('backend', 0)]

So chaning the return type of the function with the name _get*DeviceInfo* look wrong. Or am I missing something?

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        be open.                       fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError
  2013-05-29  5:47   ` Philipp Hahn
@ 2013-06-05 18:52     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-06-05 18:52 UTC (permalink / raw)
  To: Philipp Hahn; +Cc: xen-devel

On Wed, May 29, 2013 at 07:47:11AM +0200, Philipp Hahn wrote:
> Hello,
> 
> Am Dienstag 28 Mai 2013, 17:25:52 schrieben Sie:
> > >     def _getDeviceInfo_vbd(self, devid):
> > >         for dev_type, dev_info in self.info.all_devices_sxpr()
> ...
> > >                 return dev_info
> > 
> > Ewwww, that looks buggy. Could you just do 'return dev' ?
> 
> I don't think that this would be correct, since dev and devid are just the device numbers, but dev_info contains the device details  as an Sxpr:
> devid='768'
> dev='hda:cdrom' ... dev='hda' ... dev=768
> dev_type='vbd'
> dev_info=['vbd', ('uuid', '920a5c24-28c6-6abb-ad47-9c1144012986'), ('bootable', 1), ('driver', 'paravirtualised'), ('dev', 'hda:cdrom'), ('uname', 'file:/var/lib/libvirt/images/UCS_3.1-1-amd64.iso'), ('mode', 'r'), ('VDI', ''), ('backend', 0)]
> 
> So chaning the return type of the function with the name _get*DeviceInfo* look wrong. Or am I missing something?

I think you are right. My understanding from your description was incorrect.

Could you repost the patch with your SoB please? I seem to have lost it
somewhere :-( (The original email).
> 
> Sincerely
> Philipp
> -- 
> Philipp Hahn           Open Source Software Engineer      hahn@univention.de
> Univention GmbH        be open.                       fon: +49 421 22 232- 0
> Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
>                                                    http://www.univention.de/

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-06-05 18:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-24 17:53 [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError Philipp Hahn
2013-05-28 15:25 ` Konrad Rzeszutek Wilk
2013-05-29  5:47   ` Philipp Hahn
2013-06-05 18:52     ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).