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

[Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time



Hi Simon,
Did you test the patch (c/s 19679) with 'static assignment'?
It breaks the static assignment of devices...

e.g., I only place a pci=['01:00.0'] in my hvm guest config file, but I find 
ioemu invokes register_real_device() twice for the same device 01:00.0!

I haven't looked into it carefully. Just post here FYI first.

BTW, there is another bug with guest hotplug:
After I create a guest without assigning any device to it, I can 'xm 
pci-attach' a device into the guest, but "xm pci-list" doesn't show the vslot 
info. Looks this issue is originally introduced by c/s 19510.
Can you and Masaki Kanno have a look?


Thanks,
-- Dexuan


Xen patchbot-unstable wrote:
> # HG changeset patch
> # User Keir Fraser <keir.fraser@xxxxxxxxxx>
> # Date 1243585922 -3600
> # Node ID ec2bc4b9fa326048fefa81e3399e519e3902e15d
> # Parent  ef6911934b6f7c85d51417455156466ff0507a56
> xend: hot-plug PCI devices at boot-time
>
> Currently there are two interfaces to pass-through PCI devices:
> 1. A method driven through per-device xenstore entries that is used at
> boot-time
> 2. An event-based method used for hot-plug.
>
> This seems somewhat redundant and makes extending the code cumbersome
> and prone to error - often the change needs to be made twice, in
> two different ways.
>
> This patch unifies PCI pass-through by using the existing event-based
> method at boot-time.
>
> Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> ---
>  tools/python/xen/xend/XendConfig.py     |   14 +++-
>  tools/python/xen/xend/XendDomainInfo.py |   67 +++++++++++-----------
>  tools/python/xen/xend/image.py          |   16 +++++
>  tools/python/xen/xend/server/pciif.py   |   94
>  +++++++++++--------------------- 4 files changed, 96 insertions(+),
> 95 deletions(-)
>
> diff -r ef6911934b6f -r ec2bc4b9fa32
> tools/python/xen/xend/XendConfig.py ---
> a/tools/python/xen/xend/XendConfig.py Fri May 29 09:29:58 2009 +0100
> +++ b/tools/python/xen/xend/XendConfig.py     Fri May 29 09:32:02 2009
> +0100 @@ -1669,6 +1669,13 @@ class XendConfig(dict):
>
>          return ''
>
> +    def pci_convert_dict_to_sxp(self, dev, state, sub_state = None):
> +        sxp =  ['pci', ['dev'] + map(lambda (x, y): [x, y],
> dev.items()), +                ['state', state]]
> +        if sub_state != None:
> +            sxp.append(['sub_state', sub_state])
> +        return sxp
> +
>      def pci_convert_sxp_to_dict(self, dev_sxp):
>          """Convert pci device sxp to dict
>          @param dev_sxp: device configuration
> @@ -1723,9 +1730,10 @@ class XendConfig(dict):
>                      pci_dev_info[opt] = val
>                  except (TypeError, ValueError):
>                      pass
> -            # append uuid for each pci device.
> -            dpci_uuid = pci_dev_info.get('uuid', uuid.createString())
> -            pci_dev_info['uuid'] = dpci_uuid
> +            # append uuid to each pci device that does't already
> have one. +            if not pci_dev_info.has_key('uuid'):
> +                dpci_uuid = pci_dev_info.get('uuid',
> uuid.createString()) +                pci_dev_info['uuid'] = dpci_uuid
>              pci_devs.append(pci_dev_info)
>          dev_config['devs'] = pci_devs
>
> diff -r ef6911934b6f -r ec2bc4b9fa32
> tools/python/xen/xend/XendDomainInfo.py ---
> a/tools/python/xen/xend/XendDomainInfo.py     Fri May 29 09:29:58 2009
> +0100 +++ b/tools/python/xen/xend/XendDomainInfo.py   Fri May 29
>          09:32:02 2009 +0100 @@ -601,7 +601,7 @@ class
>          XendDomainInfo: asserts.isCharConvertible(key)
> self.storeDom("control/sysrq", '%c' % key)
>
> -    def sync_pcidev_info(self):
> +    def pci_device_configure_boot(self):
>
>          if not self.info.is_hvm():
>              return
> @@ -615,33 +615,12 @@ class XendDomainInfo:
>          dev_uuid = sxp.child_value(dev_info, 'uuid')
>          pci_conf = self.info['devices'][dev_uuid][1]
>          pci_devs = pci_conf['devs']
> -
> -        count = 0
> -        vslots = None
> -        while vslots is None and count < 20:
> -            vslots =
> xstransact.Read("/local/domain/0/backend/pci/%u/%s/vslots"
> -                              % (self.getDomid(), devid))
> -            time.sleep(0.1)
> -            count += 1
> -        if vslots is None:
> -            log.error("Device model didn't tell the vslots for PCI
> device")
> -            return
> -
> -        #delete last delim
> -        if vslots[-1] == ";":
> -            vslots = vslots[:-1]
> -
> -        slot_list = vslots.split(';')
> -        if len(slot_list) != len(pci_devs):
> -            log.error("Device model's pci dev num dismatch")
> -            return
> -
> -        #update the vslot info
> -        count = 0;
> -        for x in pci_devs:
> -            x['vslot'] = slot_list[count]
> -            count += 1
> -
> +        request = map(lambda x:
> +                      self.info.pci_convert_dict_to_sxp(x,
> 'Initialising', +
> 'Booting'), pci_devs) +
> +        for i in request:
> +                self.pci_device_configure(i)
>
>      def hvm_pci_device_create(self, dev_config):
>          log.debug("XendDomainInfo.hvm_pci_device_create: %s"
> @@ -741,6 +720,23 @@ class XendDomainInfo:
>                      " assigned to other domain." \
>                      )% (pci_device.name, self.info['name_label'],
> pci_str))
>
> +        return self.hvm_pci_device_insert_dev(new_dev)
> +
> +    def hvm_pci_device_insert(self, dev_config):
> +        log.debug("XendDomainInfo.hvm_pci_device_insert: %s"
> +                  % scrub_password(dev_config))
> +
> +        if not self.info.is_hvm():
> +            raise VmError("hvm_pci_device_create called on non-HVM
> guest") +
> +        new_dev = dev_config['devs'][0]
> +
> +        return self.hvm_pci_device_insert_dev(new_dev)
> +
> +    def hvm_pci_device_insert_dev(self, new_dev):
> +        log.debug("XendDomainInfo.hvm_pci_device_insert_dev: %s"
> +                  % scrub_password(new_dev))
> +
>          if self.domid is not None:
>              opts = ''
>              if 'opts' in new_dev and len(new_dev['opts']) > 0:
> @@ -752,7 +748,10 @@ class XendDomainInfo:
>                  new_dev['bus'],
>                  new_dev['slot'],
>                  new_dev['func'],
> -                new_dev['requested_vslot'],
> +                # vslot will be used when activating a
> +                # previously activated domain.
> +                # Otherwise requested_vslot will be used.
> +                assigned_or_requested_vslot(new_dev),
>                  opts)
>              self.image.signalDeviceModel('pci-ins', 'pci-inserted',
> bdf_str)
>
> @@ -827,6 +826,7 @@ class XendDomainInfo:
>              return False
>
>          pci_state = sxp.child_value(dev_sxp, 'state')
> +        pci_sub_state = sxp.child_value(dev_sxp, 'sub_state')
>          existing_dev_info = self._getDeviceInfo_pci(devid)
>
>          if existing_dev_info is None and pci_state != 'Initialising':
> @@ -840,7 +840,10 @@ class XendDomainInfo:
>          if self.info.is_hvm():
>              if pci_state == 'Initialising':
>                  # HVM PCI device attachment
> -                vslot = self.hvm_pci_device_create(dev_config)
> +                if pci_sub_state == 'Booting':
> +                    vslot = self.hvm_pci_device_insert(dev_config)
> +                else:
> +                    vslot = self.hvm_pci_device_create(dev_config)
>                  # Update vslot
>                  dev['vslot'] = vslot
>                  for n in sxp.children(pci_dev):
> @@ -907,7 +910,7 @@ class XendDomainInfo:
>                          continue
>                  new_dev_sxp.append(cur_dev)
>
> -            if pci_state == 'Initialising':
> +            if pci_state == 'Initialising' and pci_sub_state !=
>                  'Booting': for new_dev in sxp.children(dev_sxp,
>                      'dev'): new_dev_sxp.append(new_dev)
>
> @@ -2246,7 +2249,7 @@ class XendDomainInfo:
>              self.image.createDeviceModel()
>
>          #if have pass-through devs, need the virtual pci slots info
> from qemu -        self.sync_pcidev_info()
> +        self.pci_device_configure_boot()
>
>      def _releaseDevices(self, suspend = False):
>          """Release all domain's devices.  Nothrow guarantee."""
> diff -r ef6911934b6f -r ec2bc4b9fa32 tools/python/xen/xend/image.py
> --- a/tools/python/xen/xend/image.py  Fri May 29 09:29:58 2009 +0100
> +++ b/tools/python/xen/xend/image.py  Fri May 29 09:32:02 2009 +0100
> @@ -454,8 +454,22 @@ class ImageHandler:
>          if cmd is '' or ret is '':
>              raise VmError('need valid command and result when signal
> device model')
>
> -        orig_state =
> xstransact.Read("/local/domain/0/device-model/%i/state" +
> count = 0 +        while True:
> +            orig_state =
>
> xstransact.Read("/local/domain/0/device-model/%i/state" %
> self.vm.getDomid()) +            # This can occur right after
> start-up +            if orig_state != None: +                break
> +
> +            log.debug('signalDeviceModel: orig_state is None,
> retrying') +
> +            time.sleep(0.1)
> +            count += 1
> +            if count < 100:
> +                continue
> +
> +            VmError('Device model isn\'t ready for commands')
>
>          if par is not None:
>              xstransact.Store("/local/domain/0/device-model/%i"
> diff -r ef6911934b6f -r ec2bc4b9fa32
> tools/python/xen/xend/server/pciif.py ---
> a/tools/python/xen/xend/server/pciif.py       Fri May 29 09:29:58 2009
> +0100 +++ b/tools/python/xen/xend/server/pciif.py     Fri May 29 09:32:02
>          2009 +0100 @@ -69,12 +69,7 @@ class
>          PciController(DevController): """@see
>          DevController.getDeviceDetails""" back = {} pcidevid = 0
> -        vslots = ""
>          for pci_config in config.get('devs', []):
> -            attached_vslot = pci_config.get('vslot')
> -            if attached_vslot is not None:
> -                vslots = vslots + attached_vslot + ";"
> -
>              domain = parse_hex(pci_config.get('domain', 0))
>              bus = parse_hex(pci_config.get('bus', 0))
>              slot = parse_hex(pci_config.get('slot', 0))
> @@ -92,9 +87,6 @@ class PciController(DevController):
>              back['uuid-%i' % pcidevid] = pci_config.get('uuid', '')
>              back['vslot-%i' % pcidevid] = "%02x" % vslot
>              pcidevid += 1
> -
> -        if vslots != "":
> -            back['vslots'] = vslots
>
>          back['num_devs']=str(pcidevid)
>          back['uuid'] = config.get('uuid','')
> @@ -105,16 +97,17 @@ class PciController(DevController):
>
>          return (0, back, {})
>
> +    def reconfigureDevice_find(self, devid, nsearch_dev, match_dev):
> +        for j in range(nsearch_dev):
> +            if match_dev == self.readBackend(devid, 'dev-%i' % j):
> +                return j
> +        return None
>
>      def reconfigureDevice(self, _, config):
>          """@see DevController.reconfigureDevice"""
>          (devid, back, front) = self.getDeviceDetails(config)
>          num_devs = int(back['num_devs'])
>          states = config.get('states', [])
> -
> -        old_vslots = self.readBackend(devid, 'vslots')
> -        if old_vslots is None:
> -            old_vslots = ''
>          num_olddevs = int(self.readBackend(devid, 'num_devs'))
>
>          for i in range(num_devs):
> @@ -129,11 +122,15 @@ class PciController(DevController):
>                  raise XendError('Error reading config')
>
>              if state == 'Initialising':
> -                # PCI device attachment
> -                for j in range(num_olddevs):
> -                    if dev == self.readBackend(devid, 'dev-%i' % j):
> -                        raise XendError('Device %s is already
> connected.' % dev)
> -                log.debug('Attaching PCI device %s.' % dev)
> +                devno = self.reconfigureDevice_find(devid,
> num_olddevs, dev) +                if devno == None:
> +                    devno = num_olddevs + i
> +                    log.debug('Attaching PCI device %s.' % dev)
> +                    attaching = True
> +                else:
> +                    log.debug('Reconfiguring PCI device %s.' % dev)
> +                    attaching = False
> +
>                  (domain, bus, slotfunc) = dev.split(':')
>                  (slot, func) = slotfunc.split('.')
>                  domain = parse_hex(domain)
> @@ -142,41 +139,28 @@ class PciController(DevController):
>                  func = parse_hex(func)
>                  self.setupOneDevice(domain, bus, slot, func)
>
> -                self.writeBackend(devid, 'dev-%i' % (num_olddevs +
> i), dev)
> -                self.writeBackend(devid, 'state-%i' % (num_olddevs +
> i), +                self.writeBackend(devid, 'dev-%i' % devno, dev)
> +                self.writeBackend(devid, 'state-%i' % devno,
>                                    str(xenbusState['Initialising']))
> -                self.writeBackend(devid, 'uuid-%i' % (num_olddevs +
> i), uuid) +                self.writeBackend(devid, 'uuid-%i' %
>                  devno, uuid) if len(opts) > 0:
> -                    self.writeBackend(devid, 'opts-%i' %
> (num_olddevs + i), opts)
> -                self.writeBackend(devid, 'num_devs', str(num_olddevs
> + i + 1)) -
> -                # Update vslots
> -                if back['vslots'] is not None:
> -                    vslots = old_vslots + back['vslots']
> -                    self.writeBackend(devid, 'vslots', vslots)
> +                    self.writeBackend(devid, 'opts-%i' % devno, opts)
> +                if back.has_key('vslot-%i' % i):
> +                    self.writeBackend(devid, 'vslot-%i' % devno,
> +                                      back['vslot-%i' % i])
> +
> +                # If a device is being attached then num_devs will
> grow +                if attaching:
> +                    self.writeBackend(devid, 'num_devs', str(devno +
> 1))
>
>              elif state == 'Closing':
>                  # PCI device detachment
> -                found = False
> -                for j in range(num_olddevs):
> -                    if dev == self.readBackend(devid, 'dev-%i' % j):
> -                        found = True
> -                        log.debug('Detaching device %s' % dev)
> -                        self.writeBackend(devid, 'state-%i' % j,
> -
> str(xenbusState['Closing']))
> -                if not found:
> +                devno = self.reconfigureDevice_find(devid,
> num_olddevs, dev) +                if devno == None:
>                      raise XendError('Device %s is not connected' %
> dev) -
> -                # Update vslots
> -                if back.get('vslots') is not None:
> -                    vslots = old_vslots
> -                    for vslot in back['vslots'].split(';'):
> -                        if vslot != '':
> -                            vslots = vslots.replace(vslot + ';', '',
> 1)
> -                    if vslots == '':
> -                        self.removeBackend(devid, 'vslots')
> -                    else:
> -                        self.writeBackend(devid, 'vslots', vslots)
> +                log.debug('Detaching device %s' % dev)
> +                self.writeBackend(devid, 'state-%i' % devno,
> +                                  str(xenbusState['Closing']))
>
>              else:
>                  raise XendError('Error configuring device %s:
> invalid state %s' @@ -191,12 +175,6 @@ class
>          PciController(DevController): result =
>          DevController.getDeviceConfiguration(self, devid,
>          transaction) num_devs = self.readBackend(devid, 'num_devs')
> pci_devs = [] -
> -        vslots = self.readBackend(devid, 'vslots')
> -        if vslots is not None:
> -            if vslots[-1] == ";":
> -                vslots = vslots[:-1]
> -            slot_list = vslots.split(';')
>
>          for i in range(int(num_devs)):
>              dev_config = self.readBackend(devid, 'dev-%d' % i)
> @@ -215,13 +193,11 @@ class PciController(DevController):
>
>                  # Per device uuid info
>                  dev_dict['uuid'] = self.readBackend(devid, 'uuid-%d'
> % i) -
> -                #append vslot info
> -                if vslots is not None:
> -                    try:
> -                        dev_dict['vslot'] = slot_list[i]
> -                    except IndexError:
> -                        dev_dict['vslot'] = AUTO_PHP_SLOT_STR
> +                vslot = self.readBackend(devid, 'vslot-%d' % i)
> +                if vslot != None:
> +                    dev_dict['vslot'] = self.readBackend(devid,
> 'vslot-%d' % i) +                else:
> +                    dev_dict['vslot'] = AUTO_PHP_SLOT_STR
>
>                  #append opts info
>                  opts = self.readBackend(devid, 'opts-%d' % i)
>
> _______________________________________________
> Xen-changelog mailing list
> Xen-changelog@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-changelog


_______________________________________________
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®.