[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: [PATCH]xend: pass-through: fix "xm pci-list-assignable-devices' for pv_guest



On Tue, Jul 28, 2009 at 05:06:43PM +0800, Cui, Dexuan wrote:
> 
> xc.test_assign_device(dev) can only tell us if dev is assigned to hvm guest;
> if a device is assigned to pv guest, xc.test_assign_device() still tells us
> the device is not assigned yet, as a result, the device still appears in the
> output of "xm pci-list-assignable-devices", and xend would not prevent us from
> assigning the device to a new pv or hvm guest.
> 
> To judge if a device has been assigned to guest, we have to scan xenstore.  
> 
> Thanks,
> -- Dexuan
> 
> xend: pass-through: fix "xm pci-list-assignable-devices' for pv_guest
> 
> xc.test_assign_device(dev) can only tell us if dev is assigned to hvm guest;
> if a device is assigned to pv guest, xc.test_assign_device() still tells us
> the device is not assigned yet, as a result, the device still appears in the
> output of "xm pci-list-assignable-devices", and xend would not prevent us from
> assigning the device to a new pv or hvm guest.
> 
> To judge if a device has been assigned to guest, we have to scan xenstore.
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@xxxxxxxxx>
> 
> diff -r 8af26fef898c tools/python/xen/xend/XendConfig.py
> --- a/tools/python/xen/xend/XendConfig.py     Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendConfig.py     Tue Jul 28 16:42:21 2009 +0800
> @@ -2059,9 +2059,6 @@ class XendConfig(dict):
>          return self['platform'].get('hap', 0)
>  
>      def update_platform_pci(self):
> -        if not self.is_hvm():
> -            return
> -
>          pci = []
>          for dev_type, dev_info in self.all_devices_sxpr():
>              if dev_type != 'pci':
> diff -r 8af26fef898c tools/python/xen/xend/XendDomainInfo.py
> --- a/tools/python/xen/xend/XendDomainInfo.py Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendDomainInfo.py Tue Jul 28 16:42:21 2009 +0800
> @@ -42,7 +42,7 @@ from xen.util.pci import serialise_pci_o
>  from xen.util.pci import serialise_pci_opts, pci_opts_list_to_sxp, \
>                           pci_dict_to_bdf_str, pci_dict_to_xc_str, \
>                           pci_convert_sxp_to_dict, pci_convert_dict_to_sxp, \
> -                         pci_dict_cmp, PCI_DEVFN, PCI_SLOT, PCI_FUNC
> +                         pci_dict_cmp, PCI_DEVFN, PCI_SLOT, PCI_FUNC, 
> parse_hex
>  
>  from xen.xend import balloon, sxp, uuid, image, arch
>  from xen.xend import XendOptions, XendNode, XendConfig
> @@ -296,20 +296,11 @@ def dom_get(dom):
>          log.trace("domain_getinfo(%d) failed, ignoring: %s", dom, str(err))
>      return None
>  
> -def get_assigned_pci_devices(domid):
> -    dev_str_list = []
> -    path = '/local/domain/0/backend/pci/%u/0/' % domid
> -    num_devs = xstransact.Read(path + 'num_devs');
> -    if num_devs is None or num_devs == "":
> -        return dev_str_list
> -    num_devs = int(num_devs);
> -    for i in range(num_devs):
> -        dev_str = xstransact.Read(path + 'dev-%i' % i)
> -        dev_str_list = dev_str_list + [dev_str]
> -    return dev_str_list 
> +from xen.xend.server.pciif import parse_pci_name, PciDevice,\
> +    get_assigned_pci_devices, get_all_assigned_pci_devices
> +
>  
>  def do_FLR(domid):
> -    from xen.xend.server.pciif import parse_pci_name, PciDevice
>      dev_str_list = get_assigned_pci_devices(domid)
>  
>      for dev_str in dev_str_list:
> @@ -686,15 +677,14 @@ class XendDomainInfo:
>                      raise VmError("device is already inserted")
>  
>          # Test whether the devices can be assigned with VT-d
> -        bdf = xc.test_assign_device(0, pci_dict_to_xc_str(new_dev))
> -        if bdf != 0:
> -            if bdf == -1:
> -                raise VmError("failed to assign device: maybe the platform"
> -                              " doesn't support VT-d, or VT-d isn't enabled"
> -                              " properly?")
> -            raise VmError("fail to assign device(%s): maybe it has"
> -                          " already been assigned to other domain, or maybe"
> -                          " it doesn't exist." % 
> pci_dict_to_bdf_str(new_dev))
> +        pci_name = '%04x:%02x:%02x.%x' % \
> +          (parse_hex(new_dev['domain']),\
> +           parse_hex(new_dev['bus']),\
> +           parse_hex(new_dev['slot']),\
> +           parse_hex(new_dev['func']))

  You should be able to use pci_name = pci_dict_to_bdf_str(new_dev) here
  instead of the above 5 lines.

> +        if pci_name in get_all_assigned_pci_devices():
> +            raise VmError("failed to assign device %s that has"
> +                          " already been assigned to other domain." % 
> pci_str)

  Should pci_str be pci_name in the line above ?

>  
>          # Here, we duplicate some checkings (in some cases, we mustn't allow
>          # a device to be hot-plugged into an HVM guest) that are also done in
> @@ -732,8 +722,7 @@ class XendDomainInfo:
>          pci_device.devs_check_driver(coassignment_list)
>          assigned_pci_device_str_list = self._get_assigned_pci_devices()
>          for pci_str in coassignment_list:
> -            pci_dev = parse_pci_name(pci_str)
> -            if xc.test_assign_device(0, pci_dict_to_xc_str(pci_dev)) == 0:
> +            if not (pci_str in get_all_assigned_pci_devices()):
>                  continue
>              if not pci_str in assigned_pci_device_str_list:
>                  raise VmError(("pci: failed to pci-attach %s to domain %s" + 
> \
> @@ -859,7 +848,7 @@ class XendDomainInfo:
>          pci_dev = sxp.children(dev_sxp, 'dev')[0]
>          dev_config = pci_convert_sxp_to_dict(dev_sxp)
>          dev = dev_config['devs'][0]
> -                
> +
>          # Do HVM specific processing
>          if self.info.is_hvm():
>              if pci_state == 'Initialising':
> @@ -2454,7 +2443,8 @@ class XendDomainInfo:
>          if pci and len(pci) > 0:
>              pci = map(lambda x: x[0:4], pci)  # strip options 
>              pci_str = str(pci)
> -        if hvm and pci_str:
> +
> +        if hvm and pci_str != '':
>              bdf = xc.test_assign_device(0, pci_str)
>              if bdf != 0:
>                  if bdf == -1:
> @@ -2465,9 +2455,17 @@ class XendDomainInfo:
>                  devfn = (bdf >> 8) & 0xff
>                  dev = (devfn >> 3) & 0x1f
>                  func = devfn & 0x7
> -                raise VmError("fail to assign device(%x:%x.%x): maybe it has"
> +                raise VmError("failed to assign device(%x:%x.%x): maybe it 
> has"
>                                " already been assigned to other domain, or 
> maybe"
>                                " it doesn't exist." % (bus, dev, func))
> +
> +        # This test is done for both pv and hvm guest.
> +        for p in pci:
> +            pci_name = '%04x:%02x:%02x.%x' % \
> +                (parse_hex(p[0]), parse_hex(p[1]), parse_hex(p[2]), 
> parse_hex(p[3]))
> +            if pci_name in get_all_assigned_pci_devices():
> +                raise VmError("failed to assign device %s that has"
> +                              " already been assigned to other domain." % 
> pci_name)
>  
>          # register the domain in the list 
>          from xen.xend import XendDomain
> diff -r 8af26fef898c tools/python/xen/xend/XendNode.py
> --- a/tools/python/xen/xend/XendNode.py       Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendNode.py       Tue Jul 28 16:42:21 2009 +0800
> @@ -834,6 +834,9 @@ class XendNode:
>  
>  
>      def pciinfo(self):
> +        from xen.xend.server.pciif import get_all_assigned_pci_devices
> +        assigned_devs = get_all_assigned_pci_devices()
> +
>          # Each element of dev_list is a PciDevice
>          dev_list = PciUtil.find_all_assignable_devices()
>   
> @@ -847,11 +850,7 @@ class XendNode:
>          for dev_list in devs_list:
>              available = True
>              for d in dev_list:
> -                pci_str = '0x%x,0x%x,0x%x,0x%x' %(d.domain, d.bus, d.slot, 
> d.func)
> -                # Xen doesn't care what the domid is, so we pass 0 here...
> -                domid = 0
> -                bdf = self.xc.test_assign_device(domid, pci_str)
> -                if bdf != 0:
> +                if d.name in assigned_devs:

Nice use of d.name :-)

>                      available = False
>                      break
>              if available:
> diff -r 8af26fef898c tools/python/xen/xend/server/pciif.py
> --- a/tools/python/xen/xend/server/pciif.py   Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/server/pciif.py   Tue Jul 28 16:43:49 2009 +0800
> @@ -57,6 +57,25 @@ def parse_hex(val):
>              return val
>      except ValueError:
>          return None
> +
> +def get_assigned_pci_devices(domid):
> +    dev_str_list = []
> +    path = '/local/domain/0/backend/pci/%u/0/' % domid
> +    num_devs = xstransact.Read(path + 'num_devs');
> +    if num_devs is None or num_devs == "":
> +        return dev_str_list
> +    num_devs = int(num_devs)
> +    for i in range(num_devs):
> +        dev_str = xstransact.Read(path + 'dev-%i' % i)
> +        dev_str_list = dev_str_list + [dev_str]
> +    return dev_str_list
> +
> +def get_all_assigned_pci_devices():
> +    dom_list = xstransact.List('/local/domain')
> +    pci_str_list = []
> +    for d in dom_list:
> +        pci_str_list = pci_str_list + get_assigned_pci_devices(int(d))
> +    return pci_str_list
>  
>  class PciController(DevController):
>  
> @@ -368,11 +387,8 @@ class PciController(DevController):
>                      dev.devs_check_driver(funcs)
>                      for f in funcs:
>                          if not f in pci_str_list:
> -                            (f_dom, f_bus, f_slot, f_func) = 
> parse_pci_name(f)
> -                            f_pci_str = '0x%x,0x%x,0x%x,0x%x' % \
> -                                (f_dom, f_bus, f_slot, f_func)
>                              # f has been assigned to other guest?
> -                            if xc.test_assign_device(0, f_pci_str) != 0:
> +                            if f in get_all_assigned_pci_devices():
>                                  err_msg = 'pci: %s must be co-assigned to' + 
> \
>                                      ' the same guest with %s'
>                                  raise VmError(err_msg % (f, dev.name))
> @@ -396,9 +412,8 @@ class PciController(DevController):
>                      dev.devs_check_driver(devs_str)
>                      for s in devs_str:
>                          if not s in pci_str_list:
> -                            s_pci_str = 
> pci_dict_to_bdf_str(parse_pci_name(s))
>                              # s has been assigned to other guest?
> -                            if xc.test_assign_device(0, s_pci_str) != 0:
> +                            if s in get_all_assigned_pci_devices():
>                                  err_msg = 'pci: %s must be co-assigned to 
> the'+\
>                                      ' same guest with %s'
>                                  raise VmError(err_msg % (s, dev.name))
> 
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.