Re: [BUG, PATCH] xen-4.1-3 xend/XendDomainInfo.py#device_configure() TypeError

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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