WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI device

To: Simon Horman <horms@xxxxxxxxxxxx>, Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx>
Subject: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
From: "Cui, Dexuan" <dexuan.cui@xxxxxxxxx>
Date: Tue, 2 Jun 2009 13:05:16 +0800
Accept-language: zh-CN, en-US
Acceptlanguage: zh-CN, en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 01 Jun 2009 22:06:44 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <200905300930.n4U9UOTG008828@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <200905300930.n4U9UOTG008828@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcnhCYEHc8mntnyZTnqLoOP5+dy/7gCM+rrA
Thread-topic: [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