All of lore.kernel.org
 help / color / mirror / Atom feed
* RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
       [not found] <200905300930.n4U9UOTG008828@xenbits.xensource.com>
@ 2009-06-02  5:05 ` Cui, Dexuan
  2009-06-02  5:16   ` Simon Horman
  2009-06-12  5:51 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
  1 sibling, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-02  5:05 UTC (permalink / raw)
  To: Simon Horman, Masaki Kanno; +Cc: xen-devel

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@citrix.com>
> # 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@verge.net.au>
> ---
>  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@lists.xensource.com
> http://lists.xensource.com/xen-changelog

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

* Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02  5:05 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
@ 2009-06-02  5:16   ` Simon Horman
  2009-06-02  8:38     ` Cui, Dexuan
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-06-02  5:16 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Masaki Kanno

Hi Dexan,

On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> 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.

Yes, I did test static assignment, and it does seem to work.
Do you have the qemu-xen patch applied?

http://lists.xensource.com/archives/html/xen-devel/2009-05/msg01289.html

> 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?

Yes, of course, I will look into it.

Which revisions of xen-unstable.hg and qemu-xen-unstable.git are you using?

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

* RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02  5:16   ` Simon Horman
@ 2009-06-02  8:38     ` Cui, Dexuan
  2009-06-02  9:51       ` Keir Fraser
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-02  8:38 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel, Masaki Kanno

Simon Horman wrote:
> Hi Dexan,
> 
> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
>> 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.
> 
> Yes, I did test static assignment, and it does seem to work.
> Do you have the qemu-xen patch applied?
> 
> http://lists.xensource.com/archives/html/xen-devel/2009-05/msg01289.html
Oh, sorry, I didn't notice the mail... and the patch hasn't been in qemu-xen-unstable.git tree yet.
After I applied the patch (I met with a small conflict when applying it and I manually fixed it -- I guess you may need to rebase the patch against the latest qemu-xen-unstable.git), looks everything backs to normal. :-)

> 
>> 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?  
> 
> Yes, of course, I will look into it.
> 
> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are you
> using? 
I'm using the latest xen c/s 19696 and ioemu 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch manually now and can still reproduce it.

Thanks,
-- Dexuan

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02  8:38     ` Cui, Dexuan
@ 2009-06-02  9:51       ` Keir Fraser
  2009-06-02 10:02         ` Ian Jackson
  2009-06-02 11:18       ` Simon Horman
  2009-06-02 21:07       ` Information about xm list! Ata Bohra
  2 siblings, 1 reply; 26+ messages in thread
From: Keir Fraser @ 2009-06-02  9:51 UTC (permalink / raw)
  To: Cui, Dexuan, Simon Horman, Ian Jackson; +Cc: xen-devel, Masaki Kanno

On 02/06/2009 09:38, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:

>> Yes, I did test static assignment, and it does seem to work.
>> Do you have the qemu-xen patch applied?
>> 
>> http://lists.xensource.com/archives/html/xen-devel/2009-05/msg01289.html
> Oh, sorry, I didn't notice the mail... and the patch hasn't been in
> qemu-xen-unstable.git tree yet.
> After I applied the patch (I met with a small conflict when applying it and I
> manually fixed it -- I guess you may need to rebase the patch against the
> latest qemu-xen-unstable.git), looks everything backs to normal. :-)

I think the patch should be in the qemu tree already. Presumably in the
staging tree, if there is such a thing for our qemu repo?

 -- Keir

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02  9:51       ` Keir Fraser
@ 2009-06-02 10:02         ` Ian Jackson
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Jackson @ 2009-06-02 10:02 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Masaki Kanno, Simon Horman, xen-devel, Cui, Dexuan

Keir Fraser writes ("Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time"):
> I think the patch should be in the qemu tree already. Presumably in the
> staging tree, if there is such a thing for our qemu repo?

Yes, there is a staging tree for qemu and it should be in there.
Commit id 5beedb58147cbb04e206a71429198b6316217cfc.

<fx: double-checks>

Umm, it looks like I may have forgotten to push it!

Ian.

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

* Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02  8:38     ` Cui, Dexuan
  2009-06-02  9:51       ` Keir Fraser
@ 2009-06-02 11:18       ` Simon Horman
  2009-06-02 11:23         ` Cui, Dexuan
  2009-06-02 11:31         ` Cui, Dexuan
  2009-06-02 21:07       ` Information about xm list! Ata Bohra
  2 siblings, 2 replies; 26+ messages in thread
From: Simon Horman @ 2009-06-02 11:18 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Masaki Kanno

On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> Simon Horman wrote:
> > On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:

[snip]

> >> 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?  
> > 
> > Yes, of course, I will look into it.
> > 
> > Which revisions of xen-unstable.hg and qemu-xen-unstable.git are you
> > using? 
> I'm using the latest xen c/s 19696 and ioemu 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch manually now and can still reproduce it.

Are you booting an HVM domain?
I am not able to see this problem with the config below
and running the following commands to attach a device:

$ xm pci-list debian
$ xm pci-attach debian 01:00.0
$ xm pci-list debian
domain bus  slot func
0x0000 0x01 0x00 0x0

--------------------------------------

import os, re
arch = os.uname()[4]
if re.search('64', arch):
    arch_libdir = 'lib64'
else:
    arch_libdir = 'lib'
kernel = "/usr/lib/xen/boot/hvmloader"
builder='hvm'
memory = 128
name = "debian"
disk = [ 'file:/home/horms/projects/xen/media/debian-unstable.disk,hda,w',
         'file:/home/horms/projects/xen/media/debian-unstable-mini.iso,hdc:cdrom,r' ]
boot='c'
device_model = '/usr/' + arch_libdir + '/xen/bin/qemu-dm'
sdl=0
opengl=0
vnc=1
vnclisten="0.0.0.0"
vncunused=1
nographic=0
stdvga=0
serial='pty'

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

* RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 11:18       ` Simon Horman
@ 2009-06-02 11:23         ` Cui, Dexuan
  2009-06-02 11:31         ` Cui, Dexuan
  1 sibling, 0 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-02 11:23 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel, Masaki Kanno

Simon Horman wrote:
> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
>> Simon Horman wrote:
>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> 
> [snip]
> 
>>>> 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?
>>> 
>>> Yes, of course, I will look into it.
>>> 
>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are you
>>> using?
>> I'm using the latest xen c/s 19696 and ioemu
>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
>> manually now and can still reproduce it.  
> 
> Are you booting an HVM domain?
> I am not able to see this problem with the config below
> and running the following commands to attach a device:
> 
> $ xm pci-list debian
> $ xm pci-attach debian 01:00.0
> $ xm pci-list debian
> domain bus  slot func
> 0x0000 0x01 0x00 0x0 
Here, I think the VSLT is missing? 



Thanks,
-- Dexuan

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

* RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 11:18       ` Simon Horman
  2009-06-02 11:23         ` Cui, Dexuan
@ 2009-06-02 11:31         ` Cui, Dexuan
  2009-06-02 12:52           ` Simon Horman
  1 sibling, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-02 11:31 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel, Masaki Kanno

Cui, Dexuan wrote:
> Simon Horman wrote:
>> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
>>> Simon Horman wrote:
>>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
>> 
>> [snip]
>> 
>>>>> 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?
>>>> 
>>>> Yes, of course, I will look into it.
>>>> 
>>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
>>>> you using?
>>> I'm using the latest xen c/s 19696 and ioemu
>>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
>>> manually now and can still reproduce it.
>> 
>> Are you booting an HVM domain?
>> I am not able to see this problem with the config below
>> and running the following commands to attach a device:
>> 
>> $ xm pci-list debian
>> $ xm pci-attach debian 01:00.0
>> $ xm pci-list debian
>> domain bus  slot func
>> 0x0000 0x01 0x00 0x0
> Here, I think the VSLT is missing?

I expect it should be something like:

$ xm pci-list debian
VSlt domain bus  slot func
0x04 0x0000 0x01 0x00 0x0

Thanks,
-- Dexuan

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

* Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 11:31         ` Cui, Dexuan
@ 2009-06-02 12:52           ` Simon Horman
  2009-06-02 15:21             ` Simon Horman
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-06-02 12:52 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Masaki Kanno

On Tue, Jun 02, 2009 at 07:31:35PM +0800, Cui, Dexuan wrote:
> Cui, Dexuan wrote:
> > Simon Horman wrote:
> >> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> >>> Simon Horman wrote:
> >>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> >> 
> >> [snip]
> >> 
> >>>>> 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?
> >>>> 
> >>>> Yes, of course, I will look into it.
> >>>> 
> >>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
> >>>> you using?
> >>> I'm using the latest xen c/s 19696 and ioemu
> >>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
> >>> manually now and can still reproduce it.
> >> 
> >> Are you booting an HVM domain?
> >> I am not able to see this problem with the config below
> >> and running the following commands to attach a device:
> >> 
> >> $ xm pci-list debian
> >> $ xm pci-attach debian 01:00.0
> >> $ xm pci-list debian
> >> domain bus  slot func
> >> 0x0000 0x01 0x00 0x0
> > Here, I think the VSLT is missing?
> 
> I expect it should be something like:
> 
> $ xm pci-list debian
> VSlt domain bus  slot func
> 0x04 0x0000 0x01 0x00 0x0

Sorry, my mistake. I see the problem now. I will investigate.

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

* Re: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 12:52           ` Simon Horman
@ 2009-06-02 15:21             ` Simon Horman
  2009-06-03  2:50               ` Simon Horman
  2009-06-03  3:09               ` Cui, Dexuan
  0 siblings, 2 replies; 26+ messages in thread
From: Simon Horman @ 2009-06-02 15:21 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Masaki Kanno

On Tue, Jun 02, 2009 at 10:52:39PM +1000, Simon Horman wrote:
> On Tue, Jun 02, 2009 at 07:31:35PM +0800, Cui, Dexuan wrote:
> > Cui, Dexuan wrote:
> > > Simon Horman wrote:
> > >> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> > >>> Simon Horman wrote:
> > >>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> > >> 
> > >> [snip]
> > >> 
> > >>>>> 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?
> > >>>> 
> > >>>> Yes, of course, I will look into it.
> > >>>> 
> > >>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
> > >>>> you using?
> > >>> I'm using the latest xen c/s 19696 and ioemu
> > >>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
> > >>> manually now and can still reproduce it.
> > >> 
> > >> Are you booting an HVM domain?
> > >> I am not able to see this problem with the config below
> > >> and running the following commands to attach a device:
> > >> 
> > >> $ xm pci-list debian
> > >> $ xm pci-attach debian 01:00.0
> > >> $ xm pci-list debian
> > >> domain bus  slot func
> > >> 0x0000 0x01 0x00 0x0
> > > Here, I think the VSLT is missing?
> > 
> > I expect it should be something like:
> > 
> > $ xm pci-list debian
> > VSlt domain bus  slot func
> > 0x04 0x0000 0x01 0x00 0x0
> 
> Sorry, my mistake. I see the problem now. I will investigate.

Hi Dexuan,

can you see if the following resolves the problem that you are seeing?

----------------------------------------------------------------------

xend: pass-through: record the vslot of first pass-through device

Make sure that if a vslot is assigned to the first pass-through device
it is recorded and subsequently used by xm and xend.

e.g.:
$ xm pci-list debian
$ xm pci-attach debian 01:00.0
$ xm pci-list debian
VSlt domain bus  slot func
0x04 0x0000 0x01 0x00 0x0

Without this change the output of the last command is:
domain bus  slot func
0x0000 0x01 0x00 0x0

Thanks to Dexuan Cui for pointing this out. It appears to
be a regression introduced in change-set 'xm, xend: Replace "vslt" with "vslot"'
(19510:5c69f98c348e) and thus present in the 3.4.0 release.

Cc: Dexuan Cui <dexuan.cui@intel.com>,
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 01:01:01.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 01:02:03.000000000 +1000
@@ -762,6 +762,13 @@ class XendDomainInfo:
                 raise VmError(("Cannot pass-through PCI function '%s'. " +
                                "Device model reported an error: %s") %
                               (bdf_str, vslot))
+
+            # A vslot has been assigned if the result isn't AUTO_PHP_SLOT
+            # and the request contained 'requested_vslot'.
+            # If assignment has occured, update new_dev accordingly.
+            if vslot_int != AUTO_PHP_SLOT and 'requested_vslot' in new_dev:
+                new_dev['vslot'] = vslot
+                del new_dev['requested_vslot']
         else:
             vslot = new_dev['requested_vslot']
 
@@ -880,7 +887,9 @@ class XendDomainInfo:
 
         # If pci platform does not exist, create and exit.
         if existing_dev_info is None:
-            self.device_create(dev_sxp)
+            new_dev_sxp = self.info.pci_convert_dict_to_sxp(
+                    dev_config['devs'][0], pci_state, pci_sub_state)
+            self.device_create(new_dev_sxp)
             return True
 
         if self.domid is not None:

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

* Information about xm list!
  2009-06-02  8:38     ` Cui, Dexuan
  2009-06-02  9:51       ` Keir Fraser
  2009-06-02 11:18       ` Simon Horman
@ 2009-06-02 21:07       ` Ata Bohra
  2 siblings, 0 replies; 26+ messages in thread
From: Ata Bohra @ 2009-06-02 21:07 UTC (permalink / raw)
  To: xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 965 bytes --]


Dear All, 
I am trying to understand the code that gives the time spent on CPU by each domain that is printed on console by executing 'xm list'. My intention is to get the CPU usage of each domain. I have tried the xentop.c but there it gives me the potential usage of CPU for either dom0 or any domU but not the actual usage. The code for xm is all listed in python which I am not so comfortable with, it would help if I can get hold of an source tree chain to better understand the flow.
Please suggest me the source tree that I should start browsing. I have been able to find the calls that measures the idle time of the cpu (xc_getcpuinfo()) but am not sure if I will be able to get that on per domain basis.
I am using paravirtualized setup of xen.
Regards,Ata
_________________________________________________________________
Missed any of the IPL matches ? Catch a recap of all the action on MSN Videos
http://msnvideos.in/iplt20/msnvideoplayer.aspx

[-- Attachment #1.2: Type: text/html, Size: 1195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

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

* Re: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 15:21             ` Simon Horman
@ 2009-06-03  2:50               ` Simon Horman
  2009-06-03  5:40                 ` Cui, Dexuan
  2009-06-03  3:09               ` Cui, Dexuan
  1 sibling, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-06-03  2:50 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Masaki Kanno

On Wed, Jun 03, 2009 at 01:21:57AM +1000, Simon Horman wrote:
> On Tue, Jun 02, 2009 at 10:52:39PM +1000, Simon Horman wrote:
> > On Tue, Jun 02, 2009 at 07:31:35PM +0800, Cui, Dexuan wrote:
> > > Cui, Dexuan wrote:
> > > > Simon Horman wrote:
> > > >> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> > > >>> Simon Horman wrote:
> > > >>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> > > >> 
> > > >> [snip]
> > > >> 
> > > >>>>> 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?
> > > >>>> 
> > > >>>> Yes, of course, I will look into it.
> > > >>>> 
> > > >>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
> > > >>>> you using?
> > > >>> I'm using the latest xen c/s 19696 and ioemu
> > > >>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
> > > >>> manually now and can still reproduce it.
> > > >> 
> > > >> Are you booting an HVM domain?
> > > >> I am not able to see this problem with the config below
> > > >> and running the following commands to attach a device:
> > > >> 
> > > >> $ xm pci-list debian
> > > >> $ xm pci-attach debian 01:00.0
> > > >> $ xm pci-list debian
> > > >> domain bus  slot func
> > > >> 0x0000 0x01 0x00 0x0
> > > > Here, I think the VSLT is missing?
> > > 
> > > I expect it should be something like:
> > > 
> > > $ xm pci-list debian
> > > VSlt domain bus  slot func
> > > 0x04 0x0000 0x01 0x00 0x0
> > 
> > Sorry, my mistake. I see the problem now. I will investigate.
> 
> Hi Dexuan,
> 
> can you see if the following resolves the problem that you are seeing?

Hi Dexuan,

I have a cleaner approach to this problem than the patch I sent last night:

----------------------------------------------------------------------

Subject: [patch] xm: passthrough: remove requested_vslots, always use vslots
To: xen-devel@lists.xensource.com
Cc: Dexuan Cui <dexuan.cui@intel.com>,
	Masaki Kanno <kanno.masaki@jp.fujitsu.com>
From: Simon Horman <horms@verge.net.au>

As a result of the removal of the boot-time pass-through
protocol, requested_vslots is no longer needed.

Cc: Dexuan Cui <dexuan.cui@intel.com>,
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 tools/python/xen/util/pci.py            |   16 +---------------
 tools/python/xen/xend/XendConfig.py     |    7 +++----
 tools/python/xen/xend/XendDomainInfo.py |   21 ++++++++-------------
 tools/python/xen/xend/server/pciif.py   |    2 +-
 tools/python/xen/xm/create.py           |    2 +-
 tools/python/xen/xm/main.py             |    4 ++--
 6 files changed, 16 insertions(+), 36 deletions(-)

Index: xen-unstable.hg/tools/python/xen/util/pci.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/util/pci.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/util/pci.py	2009-06-03 12:26:44.000000000 +1000
@@ -146,20 +146,6 @@ def parse_pci_name(pci_name_string):
 
     return (domain, bus, slot, func)
 
-def assigned_or_requested_vslot(dev):
-    if isinstance(dev, types.DictType):
-        if dev.has_key("vslot"):
-            return dev["vslot"]
-        if dev.has_key("requested_vslot"):
-            return dev["requested_vslot"]
-    elif isinstance(dev, (types.ListType, types.TupleType)):
-        vslot = sxp.child_value(dev, 'vslot', None)
-        if not vslot:
-            vslot = sxp.child_value(dev, 'requested_vslot', None)
-        if vslot:
-            return vslot
-    raise PciDeviceVslotMissing("%s" % dev)
-
 def find_sysfs_mnt():
     try:
         return utils.find_sysfs_mount()
@@ -379,7 +365,7 @@ class PciDeviceVslotMissing(Exception):
     def __init__(self,msg):
         self.message = msg
     def __str__(self):
-        return 'pci: no vslot or requested_vslot: ' + self.message
+        return 'pci: no vslot: ' + self.message
 
 class PciDevice:
     def __init__(self, domain, bus, slot, func):
Index: xen-unstable.hg/tools/python/xen/xend/XendConfig.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendConfig.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendConfig.py	2009-06-03 12:26:44.000000000 +1000
@@ -36,7 +36,6 @@ from xen.xend.xenstore.xstransact import
 from xen.xend.server.BlktapController import blktap_disk_types
 from xen.xend.server.netif import randomMAC
 from xen.util.blkif import blkdev_name_to_number, blkdev_uname_to_file
-from xen.util.pci import assigned_or_requested_vslot
 from xen.util import xsconstants
 import xen.util.auxbin
 
@@ -1288,7 +1287,7 @@ class XendConfig(dict):
                     dpci_record = {
                         'VM': self['uuid'],
                         'PPCI': ppci_uuid,
-                        'hotplug_slot': pci_dev.get('requested_vslot', 0)
+                        'hotplug_slot': pci_dev.get('vslot', 0)
                     }
 
                     dpci_opts = pci_dev.get('opts')
@@ -1858,7 +1857,7 @@ class XendConfig(dict):
                     dpci_record = {
                         'VM': self['uuid'],
                         'PPCI': ppci_uuid,
-                        'hotplug_slot': pci_dev.get('requested_vslot', 0)
+                        'hotplug_slot': pci_dev.get('vslot', 0)
                     }
 
                     dpci_opts = pci_dev.get('opts')
@@ -2131,7 +2130,7 @@ class XendConfig(dict):
                 bus = sxp.child_value(dev, 'bus')
                 slot = sxp.child_value(dev, 'slot')
                 func = sxp.child_value(dev, 'func')
-                vslot = assigned_or_requested_vslot(dev) 
+                vslot = sxp.child_value(dev, 'vslot')
                 opts = ''
                 for opt in sxp.child_value(dev, 'opts', []):
                     if opts:
Index: xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 12:26:44.000000000 +1000
@@ -39,7 +39,7 @@ from xen.util import asserts, auxbin
 from xen.util.blkif import blkdev_uname_to_file, blkdev_uname_to_taptype
 import xen.util.xsm.xsm as security
 from xen.util import xsconstants
-from xen.util.pci import assigned_or_requested_vslot, serialise_pci_opts
+from xen.util.pci import serialise_pci_opts
 
 from xen.xend import balloon, sxp, uuid, image, arch
 from xen.xend import XendOptions, XendNode, XendConfig
@@ -641,10 +641,9 @@ class XendDomainInfo:
             pci_conf = self.info['devices'][dev_uuid][1]
             pci_devs = pci_conf['devs']
             for x in pci_devs:
-                x_vslot = assigned_or_requested_vslot(x)
-                if (int(x_vslot, 16) == int(new_dev['requested_vslot'], 16) and
-                   int(x_vslot, 16) != AUTO_PHP_SLOT):
-                    raise VmError("vslot %s already have a device." % (new_dev['requested_vslot']))
+                if (int(x['vslot'], 16) == int(new_dev['vslot'], 16) and
+                   int(x['vslot'], 16) != AUTO_PHP_SLOT):
+                    raise VmError("vslot %s already have a device." % (new_dev['vslot']))
 
                 if (int(x['domain'], 16) == int(new_dev['domain'], 16) and
                    int(x['bus'], 16)    == int(new_dev['bus'], 16) and
@@ -747,17 +746,14 @@ class XendDomainInfo:
                 new_dev['bus'],
                 new_dev['slot'],
                 new_dev['func'],
-                # vslot will be used when activating a
-                # previously activated domain.
-                # Otherwise requested_vslot will be used.
-                assigned_or_requested_vslot(new_dev),
+                new_dev['vslot'],
                 opts)
             self.image.signalDeviceModel('pci-ins', 'pci-inserted', bdf_str)
 
             vslot = xstransact.Read("/local/domain/0/device-model/%i/parameter"
                                     % self.getDomid())
         else:
-            vslot = new_dev['requested_vslot']
+            vslot = new_dev['vslot']
 
         return vslot
 
@@ -859,7 +855,7 @@ class XendDomainInfo:
                          int(x['bus'], 16) == int(dev['bus'], 16) and
                          int(x['slot'], 16) == int(dev['slot'], 16) and
                          int(x['func'], 16) == int(dev['func'], 16) ):
-                        vslot = assigned_or_requested_vslot(x)
+                        vslot = x['vslot']
                         break
                 if vslot == "":
                     raise VmError("Device %04x:%02x:%02x.%01x is not connected"
@@ -1138,8 +1134,7 @@ class XendDomainInfo:
         #find the pass-through device with the virtual slot
         devnum = 0
         for x in pci_conf['devs']:
-            x_vslot = assigned_or_requested_vslot(x)
-            if int(x_vslot, 16) == vslot:
+            if int(x['vslot'], 16) == vslot:
                 break
             devnum += 1
 
Index: xen-unstable.hg/tools/python/xen/xm/create.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xm/create.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xm/create.py	2009-06-03 12:26:44.000000000 +1000
@@ -716,7 +716,7 @@ def configure_pci(config_devs, vals):
 
         config_pci_bdf = ['dev', ['domain', domain], ['bus', bus], \
                           ['slot', slot], ['func', func],
-                          ['requested_vslot', vslot]]
+                          ['vslot', vslot]]
         map(f, d.keys())
         if len(config_pci_opts)>0:
             config_pci_bdf.append(['opts', config_pci_opts])
Index: xen-unstable.hg/tools/python/xen/xm/main.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xm/main.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xm/main.py	2009-06-03 12:26:44.000000000 +1000
@@ -2198,7 +2198,7 @@ def xm_pci_list(args):
                 "bus":      int(x["bus"], 16),
                 "slot":     int(x["slot"], 16),
                 "func":     int(x["func"], 16),
-                "vslot":    int(assigned_or_requested_vslot(x), 16)
+                "vslot":    int(x["vslot"], 16)
             }
             devs.append(dev)
 
@@ -2500,7 +2500,7 @@ def parse_pci_configuration(args, state,
                 ['bus', '0x'+ pci_dev_info['bus']],
                 ['slot', '0x'+ pci_dev_info['slot']],
                 ['func', '0x'+ pci_dev_info['func']],
-                ['requested_vslot', '0x%x' % int(vslot, 16)]]
+                ['vslot', '0x%x' % int(vslot, 16)]]
         if len(opts) > 0:
             pci_bdf.append(['opts', opts])
         pci.append(pci_bdf)
Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-03 12:26:44.000000000 +1000
@@ -74,7 +74,7 @@ class PciController(DevController):
             bus = parse_hex(pci_config.get('bus', 0))
             slot = parse_hex(pci_config.get('slot', 0))
             func = parse_hex(pci_config.get('func', 0))            
-            vslot = parse_hex(assigned_or_requested_vslot(pci_config))
+            vslot = parse_hex(pci_config.get('vslot', 0))
 
             if pci_config.has_key('opts'):
                 opts = serialise_pci_opts(pci_config['opts'])

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

* RE: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-02 15:21             ` Simon Horman
  2009-06-03  2:50               ` Simon Horman
@ 2009-06-03  3:09               ` Cui, Dexuan
  1 sibling, 0 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-03  3:09 UTC (permalink / raw)
  To: Simon Horman; +Cc: Masaki, xen-devel, Kanno

Hi Simon,
I can't apply your patch --  the first blob ("@@ -762,6 +762,13 @@") can't be applied.
I'm using 19698:f72d26c00002.

Thanks,
-- Dexuan



-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?6?2? 23:22
To: Cui, Dexuan
Cc: xen-devel@lists.xensource.com; Masaki Kanno
Subject: Re: [Xen-devel] Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Tue, Jun 02, 2009 at 10:52:39PM +1000, Simon Horman wrote:
> On Tue, Jun 02, 2009 at 07:31:35PM +0800, Cui, Dexuan wrote:
> > Cui, Dexuan wrote:
> > > Simon Horman wrote:
> > >> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> > >>> Simon Horman wrote:
> > >>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> > >> 
> > >> [snip]
> > >> 
> > >>>>> 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?
> > >>>> 
> > >>>> Yes, of course, I will look into it.
> > >>>> 
> > >>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
> > >>>> you using?
> > >>> I'm using the latest xen c/s 19696 and ioemu
> > >>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
> > >>> manually now and can still reproduce it.
> > >> 
> > >> Are you booting an HVM domain?
> > >> I am not able to see this problem with the config below
> > >> and running the following commands to attach a device:
> > >> 
> > >> $ xm pci-list debian
> > >> $ xm pci-attach debian 01:00.0
> > >> $ xm pci-list debian
> > >> domain bus  slot func
> > >> 0x0000 0x01 0x00 0x0
> > > Here, I think the VSLT is missing?
> > 
> > I expect it should be something like:
> > 
> > $ xm pci-list debian
> > VSlt domain bus  slot func
> > 0x04 0x0000 0x01 0x00 0x0
> 
> Sorry, my mistake. I see the problem now. I will investigate.

Hi Dexuan,

can you see if the following resolves the problem that you are seeing?

----------------------------------------------------------------------

xend: pass-through: record the vslot of first pass-through device

Make sure that if a vslot is assigned to the first pass-through device
it is recorded and subsequently used by xm and xend.

e.g.:
$ xm pci-list debian
$ xm pci-attach debian 01:00.0
$ xm pci-list debian
VSlt domain bus  slot func
0x04 0x0000 0x01 0x00 0x0

Without this change the output of the last command is:
domain bus  slot func
0x0000 0x01 0x00 0x0

Thanks to Dexuan Cui for pointing this out. It appears to
be a regression introduced in change-set 'xm, xend: Replace "vslt" with "vslot"'
(19510:5c69f98c348e) and thus present in the 3.4.0 release.

Cc: Dexuan Cui <dexuan.cui@intel.com>,
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 01:01:01.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 01:02:03.000000000 +1000
@@ -762,6 +762,13 @@ class XendDomainInfo:
                 raise VmError(("Cannot pass-through PCI function '%s'. " +
                                "Device model reported an error: %s") %
                               (bdf_str, vslot))
+
+            # A vslot has been assigned if the result isn't AUTO_PHP_SLOT
+            # and the request contained 'requested_vslot'.
+            # If assignment has occured, update new_dev accordingly.
+            if vslot_int != AUTO_PHP_SLOT and 'requested_vslot' in new_dev:
+                new_dev['vslot'] = vslot
+                del new_dev['requested_vslot']
         else:
             vslot = new_dev['requested_vslot']
 
@@ -880,7 +887,9 @@ class XendDomainInfo:
 
         # If pci platform does not exist, create and exit.
         if existing_dev_info is None:
-            self.device_create(dev_sxp)
+            new_dev_sxp = self.info.pci_convert_dict_to_sxp(
+                    dev_config['devs'][0], pci_state, pci_sub_state)
+            self.device_create(new_dev_sxp)
             return True
 
         if self.domid is not None:

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

* RE: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-03  2:50               ` Simon Horman
@ 2009-06-03  5:40                 ` Cui, Dexuan
  2009-06-03  6:04                   ` Simon Horman
  0 siblings, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-03  5:40 UTC (permalink / raw)
  To: Simon Horman, Keir Fraser; +Cc: Masaki, xen-devel, Kanno

> I have a cleaner approach to this problem than the patch I sent last night: 

Hi Simon, 
Yes, with it, I think everything works pretty fine now!  :-)

BTW, I think we should consider backing port the related patches to the xen-3.4 and qemu-xen-3.4 trees. :-)


Thanks,
-- Dexuan

-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?6?3? 10:51
To: Cui, Dexuan
Cc: xen-devel@lists.xensource.com; Masaki Kanno
Subject: Re: [Xen-devel] Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Wed, Jun 03, 2009 at 01:21:57AM +1000, Simon Horman wrote:
> On Tue, Jun 02, 2009 at 10:52:39PM +1000, Simon Horman wrote:
> > On Tue, Jun 02, 2009 at 07:31:35PM +0800, Cui, Dexuan wrote:
> > > Cui, Dexuan wrote:
> > > > Simon Horman wrote:
> > > >> On Tue, Jun 02, 2009 at 04:38:17PM +0800, Cui, Dexuan wrote:
> > > >>> Simon Horman wrote:
> > > >>>> On Tue, Jun 02, 2009 at 01:05:16PM +0800, Cui, Dexuan wrote:
> > > >> 
> > > >> [snip]
> > > >> 
> > > >>>>> 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?
> > > >>>> 
> > > >>>> Yes, of course, I will look into it.
> > > >>>> 
> > > >>>> Which revisions of xen-unstable.hg and qemu-xen-unstable.git are
> > > >>>> you using?
> > > >>> I'm using the latest xen c/s 19696 and ioemu
> > > >>> 72f4654095e0ac1539749b628e98f5e1569c9801 plus applying your patch
> > > >>> manually now and can still reproduce it.
> > > >> 
> > > >> Are you booting an HVM domain?
> > > >> I am not able to see this problem with the config below
> > > >> and running the following commands to attach a device:
> > > >> 
> > > >> $ xm pci-list debian
> > > >> $ xm pci-attach debian 01:00.0
> > > >> $ xm pci-list debian
> > > >> domain bus  slot func
> > > >> 0x0000 0x01 0x00 0x0
> > > > Here, I think the VSLT is missing?
> > > 
> > > I expect it should be something like:
> > > 
> > > $ xm pci-list debian
> > > VSlt domain bus  slot func
> > > 0x04 0x0000 0x01 0x00 0x0
> > 
> > Sorry, my mistake. I see the problem now. I will investigate.
> 
> Hi Dexuan,
> 
> can you see if the following resolves the problem that you are seeing?

Hi Dexuan,

I have a cleaner approach to this problem than the patch I sent last night:

----------------------------------------------------------------------

Subject: [patch] xm: passthrough: remove requested_vslots, always use vslots
To: xen-devel@lists.xensource.com
Cc: Dexuan Cui <dexuan.cui@intel.com>,
	Masaki Kanno <kanno.masaki@jp.fujitsu.com>
From: Simon Horman <horms@verge.net.au>

As a result of the removal of the boot-time pass-through
protocol, requested_vslots is no longer needed.

Cc: Dexuan Cui <dexuan.cui@intel.com>,
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

 tools/python/xen/util/pci.py            |   16 +---------------
 tools/python/xen/xend/XendConfig.py     |    7 +++----
 tools/python/xen/xend/XendDomainInfo.py |   21 ++++++++-------------
 tools/python/xen/xend/server/pciif.py   |    2 +-
 tools/python/xen/xm/create.py           |    2 +-
 tools/python/xen/xm/main.py             |    4 ++--
 6 files changed, 16 insertions(+), 36 deletions(-)

Index: xen-unstable.hg/tools/python/xen/util/pci.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/util/pci.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/util/pci.py	2009-06-03 12:26:44.000000000 +1000
@@ -146,20 +146,6 @@ def parse_pci_name(pci_name_string):
 
     return (domain, bus, slot, func)
 
-def assigned_or_requested_vslot(dev):
-    if isinstance(dev, types.DictType):
-        if dev.has_key("vslot"):
-            return dev["vslot"]
-        if dev.has_key("requested_vslot"):
-            return dev["requested_vslot"]
-    elif isinstance(dev, (types.ListType, types.TupleType)):
-        vslot = sxp.child_value(dev, 'vslot', None)
-        if not vslot:
-            vslot = sxp.child_value(dev, 'requested_vslot', None)
-        if vslot:
-            return vslot
-    raise PciDeviceVslotMissing("%s" % dev)
-
 def find_sysfs_mnt():
     try:
         return utils.find_sysfs_mount()
@@ -379,7 +365,7 @@ class PciDeviceVslotMissing(Exception):
     def __init__(self,msg):
         self.message = msg
     def __str__(self):
-        return 'pci: no vslot or requested_vslot: ' + self.message
+        return 'pci: no vslot: ' + self.message
 
 class PciDevice:
     def __init__(self, domain, bus, slot, func):
Index: xen-unstable.hg/tools/python/xen/xend/XendConfig.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendConfig.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendConfig.py	2009-06-03 12:26:44.000000000 +1000
@@ -36,7 +36,6 @@ from xen.xend.xenstore.xstransact import
 from xen.xend.server.BlktapController import blktap_disk_types
 from xen.xend.server.netif import randomMAC
 from xen.util.blkif import blkdev_name_to_number, blkdev_uname_to_file
-from xen.util.pci import assigned_or_requested_vslot
 from xen.util import xsconstants
 import xen.util.auxbin
 
@@ -1288,7 +1287,7 @@ class XendConfig(dict):
                     dpci_record = {
                         'VM': self['uuid'],
                         'PPCI': ppci_uuid,
-                        'hotplug_slot': pci_dev.get('requested_vslot', 0)
+                        'hotplug_slot': pci_dev.get('vslot', 0)
                     }
 
                     dpci_opts = pci_dev.get('opts')
@@ -1858,7 +1857,7 @@ class XendConfig(dict):
                     dpci_record = {
                         'VM': self['uuid'],
                         'PPCI': ppci_uuid,
-                        'hotplug_slot': pci_dev.get('requested_vslot', 0)
+                        'hotplug_slot': pci_dev.get('vslot', 0)
                     }
 
                     dpci_opts = pci_dev.get('opts')
@@ -2131,7 +2130,7 @@ class XendConfig(dict):
                 bus = sxp.child_value(dev, 'bus')
                 slot = sxp.child_value(dev, 'slot')
                 func = sxp.child_value(dev, 'func')
-                vslot = assigned_or_requested_vslot(dev) 
+                vslot = sxp.child_value(dev, 'vslot')
                 opts = ''
                 for opt in sxp.child_value(dev, 'opts', []):
                     if opts:
Index: xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/XendDomainInfo.py	2009-06-03 12:26:44.000000000 +1000
@@ -39,7 +39,7 @@ from xen.util import asserts, auxbin
 from xen.util.blkif import blkdev_uname_to_file, blkdev_uname_to_taptype
 import xen.util.xsm.xsm as security
 from xen.util import xsconstants
-from xen.util.pci import assigned_or_requested_vslot, serialise_pci_opts
+from xen.util.pci import serialise_pci_opts
 
 from xen.xend import balloon, sxp, uuid, image, arch
 from xen.xend import XendOptions, XendNode, XendConfig
@@ -641,10 +641,9 @@ class XendDomainInfo:
             pci_conf = self.info['devices'][dev_uuid][1]
             pci_devs = pci_conf['devs']
             for x in pci_devs:
-                x_vslot = assigned_or_requested_vslot(x)
-                if (int(x_vslot, 16) == int(new_dev['requested_vslot'], 16) and
-                   int(x_vslot, 16) != AUTO_PHP_SLOT):
-                    raise VmError("vslot %s already have a device." % (new_dev['requested_vslot']))
+                if (int(x['vslot'], 16) == int(new_dev['vslot'], 16) and
+                   int(x['vslot'], 16) != AUTO_PHP_SLOT):
+                    raise VmError("vslot %s already have a device." % (new_dev['vslot']))
 
                 if (int(x['domain'], 16) == int(new_dev['domain'], 16) and
                    int(x['bus'], 16)    == int(new_dev['bus'], 16) and
@@ -747,17 +746,14 @@ class XendDomainInfo:
                 new_dev['bus'],
                 new_dev['slot'],
                 new_dev['func'],
-                # vslot will be used when activating a
-                # previously activated domain.
-                # Otherwise requested_vslot will be used.
-                assigned_or_requested_vslot(new_dev),
+                new_dev['vslot'],
                 opts)
             self.image.signalDeviceModel('pci-ins', 'pci-inserted', bdf_str)
 
             vslot = xstransact.Read("/local/domain/0/device-model/%i/parameter"
                                     % self.getDomid())
         else:
-            vslot = new_dev['requested_vslot']
+            vslot = new_dev['vslot']
 
         return vslot
 
@@ -859,7 +855,7 @@ class XendDomainInfo:
                          int(x['bus'], 16) == int(dev['bus'], 16) and
                          int(x['slot'], 16) == int(dev['slot'], 16) and
                          int(x['func'], 16) == int(dev['func'], 16) ):
-                        vslot = assigned_or_requested_vslot(x)
+                        vslot = x['vslot']
                         break
                 if vslot == "":
                     raise VmError("Device %04x:%02x:%02x.%01x is not connected"
@@ -1138,8 +1134,7 @@ class XendDomainInfo:
         #find the pass-through device with the virtual slot
         devnum = 0
         for x in pci_conf['devs']:
-            x_vslot = assigned_or_requested_vslot(x)
-            if int(x_vslot, 16) == vslot:
+            if int(x['vslot'], 16) == vslot:
                 break
             devnum += 1
 
Index: xen-unstable.hg/tools/python/xen/xm/create.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xm/create.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xm/create.py	2009-06-03 12:26:44.000000000 +1000
@@ -716,7 +716,7 @@ def configure_pci(config_devs, vals):
 
         config_pci_bdf = ['dev', ['domain', domain], ['bus', bus], \
                           ['slot', slot], ['func', func],
-                          ['requested_vslot', vslot]]
+                          ['vslot', vslot]]
         map(f, d.keys())
         if len(config_pci_opts)>0:
             config_pci_bdf.append(['opts', config_pci_opts])
Index: xen-unstable.hg/tools/python/xen/xm/main.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xm/main.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xm/main.py	2009-06-03 12:26:44.000000000 +1000
@@ -2198,7 +2198,7 @@ def xm_pci_list(args):
                 "bus":      int(x["bus"], 16),
                 "slot":     int(x["slot"], 16),
                 "func":     int(x["func"], 16),
-                "vslot":    int(assigned_or_requested_vslot(x), 16)
+                "vslot":    int(x["vslot"], 16)
             }
             devs.append(dev)
 
@@ -2500,7 +2500,7 @@ def parse_pci_configuration(args, state,
                 ['bus', '0x'+ pci_dev_info['bus']],
                 ['slot', '0x'+ pci_dev_info['slot']],
                 ['func', '0x'+ pci_dev_info['func']],
-                ['requested_vslot', '0x%x' % int(vslot, 16)]]
+                ['vslot', '0x%x' % int(vslot, 16)]]
         if len(opts) > 0:
             pci_bdf.append(['opts', opts])
         pci.append(pci_bdf)
Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-03 12:26:42.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-03 12:26:44.000000000 +1000
@@ -74,7 +74,7 @@ class PciController(DevController):
             bus = parse_hex(pci_config.get('bus', 0))
             slot = parse_hex(pci_config.get('slot', 0))
             func = parse_hex(pci_config.get('func', 0))            
-            vslot = parse_hex(assigned_or_requested_vslot(pci_config))
+            vslot = parse_hex(pci_config.get('vslot', 0))
 
             if pci_config.has_key('opts'):
                 opts = serialise_pci_opts(pci_config['opts'])

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

* Re: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-03  5:40                 ` Cui, Dexuan
@ 2009-06-03  6:04                   ` Simon Horman
  0 siblings, 0 replies; 26+ messages in thread
From: Simon Horman @ 2009-06-03  6:04 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Keir Fraser, Masaki Kanno

On Wed, Jun 03, 2009 at 01:40:06PM +0800, Cui, Dexuan wrote:
> > I have a cleaner approach to this problem than the patch I sent last night: 
> 
> Hi Simon, 
> Yes, with it, I think everything works pretty fine now!  :-)
> 
> BTW, I think we should consider backing port the related patches to the xen-3.4 and qemu-xen-3.4 trees. :-)

I expect that the first patch that I sent would be more
suitable for a backport.

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

* Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
       [not found] <200905300930.n4U9UOTG008828@xenbits.xensource.com>
  2009-06-02  5:05 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
@ 2009-06-12  5:51 ` Cui, Dexuan
  2009-06-12  6:33   ` Simon Horman
  1 sibling, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-12  5:51 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel

Hi Simon,
After this changeset, I find there are some new issues in the xend:
I noticed in xend.log, setupOneDevice() is invoked twice, but actually I only statically assign 1 device to hvm guest.

After looking into the xend code, I find in XendDomainInfo.py: _initDomain() -> _createDevices(), we invoke self._createDevice(devclass, config) that eventually invokes setupOneDevice() -- this is the first time;
And later, still in  _createDevices(), we invoke pci_device_configure_boot() -> pci_device_configure() -> dev_control.reconfigureDevice(devid, dev_config) -> xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()  --  this is the second time.  Can you remove the duplicate invocation?

Thanks,
-- Dexuan



-----Original Message-----
From: xen-changelog-bounces@lists.xensource.com [mailto:xen-changelog-bounces@lists.xensource.com] On Behalf Of Xen patchbot-unstable
Sent: 2009?5?30? 17:30
To: xen-changelog@lists.xensource.com
Subject: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# 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@verge.net.au>
---
 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@lists.xensource.com
http://lists.xensource.com/xen-changelog

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

* Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-12  5:51 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
@ 2009-06-12  6:33   ` Simon Horman
  2009-06-12  6:35     ` Cui, Dexuan
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-06-12  6:33 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel

On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> Hi Simon,
> After this changeset, I find there are some new issues in the xend:
> I noticed in xend.log, setupOneDevice() is invoked twice, but actually I only statically assign 1 device to hvm guest.
> 
> After looking into the xend code, I find in XendDomainInfo.py: _initDomain() -> _createDevices(), we invoke self._createDevice(devclass, config) that eventually invokes setupOneDevice() -- this is the first time;
> And later, still in  _createDevices(), we invoke pci_device_configure_boot() -> pci_device_configure() -> dev_control.reconfigureDevice(devid, dev_config) -> xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()  --  this is the second time.  Can you remove the duplicate invocation?

Sure, I will look into it ASAP.

Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
you are using?

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

* RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-12  6:33   ` Simon Horman
@ 2009-06-12  6:35     ` Cui, Dexuan
  2009-06-12  6:45       ` Simon Horman
  2009-06-15  1:53       ` Simon Horman
  0 siblings, 2 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-12  6:35 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel

I'm using the latest xen-unstable 19740, Dom0 898, ioemu e0bb6b8df60863bca0163a1688baf4854e931e55.

Thanks,
-- Dexuan



-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?6?12? 14:34
To: Cui, Dexuan
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> Hi Simon,
> After this changeset, I find there are some new issues in the xend:
> I noticed in xend.log, setupOneDevice() is invoked twice, but actually I only statically assign 1 device to hvm guest.
> 
> After looking into the xend code, I find in XendDomainInfo.py: _initDomain() -> _createDevices(), we invoke self._createDevice(devclass, config) that eventually invokes setupOneDevice() -- this is the first time;
> And later, still in  _createDevices(), we invoke pci_device_configure_boot() -> pci_device_configure() -> dev_control.reconfigureDevice(devid, dev_config) -> xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()  --  this is the second time.  Can you remove the duplicate invocation?

Sure, I will look into it ASAP.

Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
you are using?

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-12  6:35     ` Cui, Dexuan
@ 2009-06-12  6:45       ` Simon Horman
  2009-06-15  1:53       ` Simon Horman
  1 sibling, 0 replies; 26+ messages in thread
From: Simon Horman @ 2009-06-12  6:45 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel

On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> I'm using the latest xen-unstable 19740, Dom0 898, ioemu e0bb6b8df60863bca0163a1688baf4854e931e55.

Thanks, I'll look into it.

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-12  6:35     ` Cui, Dexuan
  2009-06-12  6:45       ` Simon Horman
@ 2009-06-15  1:53       ` Simon Horman
  2009-06-15  2:01         ` Cui, Dexuan
  2009-07-28  6:47         ` Cui, Dexuan
  1 sibling, 2 replies; 26+ messages in thread
From: Simon Horman @ 2009-06-15  1:53 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel

On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> On Fri, Jun 12, 2009 at 14:34, Simon Horman wrote:
> > On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> > > Hi Simon,
> > > After this changeset, I find there are some new issues in the xend:
> > > I noticed in xend.log, setupOneDevice() is invoked twice,
> > > but actually I only statically assign 1 device to hvm guest.
> > > 
> > > After looking into the xend code, I find in XendDomainInfo.py:
> > > _initDomain() -> _createDevices(), we invoke
> > > self._createDevice(devclass, config) that eventually invokes
> > > setupOneDevice() -- this is the first time;
> > > And later, still in  _createDevices(), we invoke
> > > pci_device_configure_boot() -> pci_device_configure() ->
> > > dev_control.reconfigureDevice(devid, dev_config) ->
> > > xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()
> > > --  this is the second time.  Can you remove the duplicate invocation?
> > 
> > Sure, I will look into it ASAP.
> 
> > Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
> > you are using?
> I'm using the latest xen-unstable 19740, Dom0 898, ioemu
> e0bb6b8df60863bca0163a1688baf4854e931e55.

Hi Dexuan,

I think that a simple solution to this is to just remove the
first invocation.

-----------------------------------------------------------------------

xend: pass-through: Only call setupOneDevice() once per device

As observed by Dexuan Cui, when PCI devices are passed through at
domain-creation-time setupOneDevice() will be called twice.

Once via setupDevice() and once via econfigureDevice() which
is called in pci_device_configure().

This patch removes the first of these.

Cc: Dexuan Cui <dexuan.cui@intel.com>
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:00.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:02.000000000 +1000
@@ -436,8 +436,6 @@ class PciController(DevController):
                                     ' same guest with %s'
                                 raise VmError(err_msg % (s, dev.name))
 
-        for (domain, bus, slot, func) in pci_dev_list:
-            self.setupOneDevice(domain, bus, slot, func)
         wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
         self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
         log.debug('pci: register aer watch %s', wPath)

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

* RE: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-15  1:53       ` Simon Horman
@ 2009-06-15  2:01         ` Cui, Dexuan
  2009-07-28  6:47         ` Cui, Dexuan
  1 sibling, 0 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-06-15  2:01 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel

Yes, this can avoid the duplication invocation. 

Thanks,
-- Dexuan



-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?6?15? 9:53
To: Cui, Dexuan
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> On Fri, Jun 12, 2009 at 14:34, Simon Horman wrote:
> > On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> > > Hi Simon,
> > > After this changeset, I find there are some new issues in the xend:
> > > I noticed in xend.log, setupOneDevice() is invoked twice,
> > > but actually I only statically assign 1 device to hvm guest.
> > > 
> > > After looking into the xend code, I find in XendDomainInfo.py:
> > > _initDomain() -> _createDevices(), we invoke
> > > self._createDevice(devclass, config) that eventually invokes
> > > setupOneDevice() -- this is the first time;
> > > And later, still in  _createDevices(), we invoke
> > > pci_device_configure_boot() -> pci_device_configure() ->
> > > dev_control.reconfigureDevice(devid, dev_config) ->
> > > xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()
> > > --  this is the second time.  Can you remove the duplicate invocation?
> > 
> > Sure, I will look into it ASAP.
> 
> > Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
> > you are using?
> I'm using the latest xen-unstable 19740, Dom0 898, ioemu
> e0bb6b8df60863bca0163a1688baf4854e931e55.

Hi Dexuan,

I think that a simple solution to this is to just remove the
first invocation.

-----------------------------------------------------------------------

xend: pass-through: Only call setupOneDevice() once per device

As observed by Dexuan Cui, when PCI devices are passed through at
domain-creation-time setupOneDevice() will be called twice.

Once via setupDevice() and once via econfigureDevice() which
is called in pci_device_configure().

This patch removes the first of these.

Cc: Dexuan Cui <dexuan.cui@intel.com>
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:00.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:02.000000000 +1000
@@ -436,8 +436,6 @@ class PciController(DevController):
                                     ' same guest with %s'
                                 raise VmError(err_msg % (s, dev.name))
 
-        for (domain, bus, slot, func) in pci_dev_list:
-            self.setupOneDevice(domain, bus, slot, func)
         wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
         self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
         log.debug('pci: register aer watch %s', wPath)

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

* RE: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-06-15  1:53       ` Simon Horman
  2009-06-15  2:01         ` Cui, Dexuan
@ 2009-07-28  6:47         ` Cui, Dexuan
  2009-07-28  7:15           ` Simon Horman
  1 sibling, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-07-28  6:47 UTC (permalink / raw)
  To: Simon Horman, Keir Fraser; +Cc: xen-devel

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

Hi Simon, 
> I think that a simple solution to this is to just remove the first invocation.
This was checked in as c/s 19754.
Unluckily, this breaks device assignment for pv guest: xend would not invoke setupOneDevice() for pv guest at all.

The attached patch fixes the issue.  Please have a look.

Thanks,
-- Dexuan



-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?6?15? 9:53
To: Cui, Dexuan
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> On Fri, Jun 12, 2009 at 14:34, Simon Horman wrote:
> > On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> > > Hi Simon,
> > > After this changeset, I find there are some new issues in the xend:
> > > I noticed in xend.log, setupOneDevice() is invoked twice,
> > > but actually I only statically assign 1 device to hvm guest.
> > > 
> > > After looking into the xend code, I find in XendDomainInfo.py:
> > > _initDomain() -> _createDevices(), we invoke
> > > self._createDevice(devclass, config) that eventually invokes
> > > setupOneDevice() -- this is the first time;
> > > And later, still in  _createDevices(), we invoke
> > > pci_device_configure_boot() -> pci_device_configure() ->
> > > dev_control.reconfigureDevice(devid, dev_config) ->
> > > xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()
> > > --  this is the second time.  Can you remove the duplicate invocation?
> > 
> > Sure, I will look into it ASAP.
> 
> > Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
> > you are using?
> I'm using the latest xen-unstable 19740, Dom0 898, ioemu
> e0bb6b8df60863bca0163a1688baf4854e931e55.

Hi Dexuan,

I think that a simple solution to this is to just remove the
first invocation.

-----------------------------------------------------------------------

xend: pass-through: Only call setupOneDevice() once per device

As observed by Dexuan Cui, when PCI devices are passed through at
domain-creation-time setupOneDevice() will be called twice.

Once via setupDevice() and once via econfigureDevice() which
is called in pci_device_configure().

This patch removes the first of these.

Cc: Dexuan Cui <dexuan.cui@intel.com>
Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
===================================================================
--- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:00.000000000 +1000
+++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:02.000000000 +1000
@@ -436,8 +436,6 @@ class PciController(DevController):
                                     ' same guest with %s'
                                 raise VmError(err_msg % (s, dev.name))
 
-        for (domain, bus, slot, func) in pci_dev_list:
-            self.setupOneDevice(domain, bus, slot, func)
         wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
         self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
         log.debug('pci: register aer watch %s', wPath)

[-- Attachment #2: add_the_missing_setupOneDevice_for_pv_guest.patch --]
[-- Type: application/octet-stream, Size: 1387 bytes --]

xend: pass-through: fix pci passthrough for pv guest

C/S 19754: a5f584c1e2f6 breaks pci passthrough for pv guest.
The patch fixes the issue.

Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>

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 14:28:14 2009 +0800
@@ -403,6 +403,17 @@ class PciController(DevController):
                                     ' same guest with %s'
                                 raise VmError(err_msg % (s, dev.name))
 
+        # Assigning device staticaly (namely, the pci string in guest config
+        # file) to PV guest needs this setupOneDevice().
+        # Assigning device dynamically (namely, 'xm pci-attach') to PV guest
+        #  would go through reconfigureDevice().
+        #
+        # For hvm guest, (from c/s 19679 on) assigning device statically and
+        # dynamically both go through reconfigureDevice(), so HERE the
+        # setupOneDevice() is not necessary.
+        if not self.vm.info.is_hvm():
+            for d in pci_dev_list:
+                self.setupOneDevice(d)
         wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
         self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
         log.debug('pci: register aer watch %s', wPath)

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

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

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-07-28  6:47         ` Cui, Dexuan
@ 2009-07-28  7:15           ` Simon Horman
  2009-07-28  8:42             ` Cui, Dexuan
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-07-28  7:15 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Keir Fraser

Hi,

is an alternative to just revert 19754 and allow
duplicate calls to setupOneDevice() in the HVM case?

On Tue, Jul 28, 2009 at 02:47:16PM +0800, Cui, Dexuan wrote:
> Hi Simon, 
> > I think that a simple solution to this is to just remove the first invocation.
> This was checked in as c/s 19754.
> Unluckily, this breaks device assignment for pv guest: xend would not invoke setupOneDevice() for pv guest at all.
> 
> The attached patch fixes the issue.  Please have a look.
> 
> Thanks,
> -- Dexuan
> 
> 
> 
> -----Original Message-----
> From: Simon Horman [mailto:horms@verge.net.au] 
> Sent: 2009?6?15? 9:53
> To: Cui, Dexuan
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
> 
> On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> > On Fri, Jun 12, 2009 at 14:34, Simon Horman wrote:
> > > On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> > > > Hi Simon,
> > > > After this changeset, I find there are some new issues in the xend:
> > > > I noticed in xend.log, setupOneDevice() is invoked twice,
> > > > but actually I only statically assign 1 device to hvm guest.
> > > > 
> > > > After looking into the xend code, I find in XendDomainInfo.py:
> > > > _initDomain() -> _createDevices(), we invoke
> > > > self._createDevice(devclass, config) that eventually invokes
> > > > setupOneDevice() -- this is the first time;
> > > > And later, still in  _createDevices(), we invoke
> > > > pci_device_configure_boot() -> pci_device_configure() ->
> > > > dev_control.reconfigureDevice(devid, dev_config) ->
> > > > xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()
> > > > --  this is the second time.  Can you remove the duplicate invocation?
> > > 
> > > Sure, I will look into it ASAP.
> > 
> > > Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
> > > you are using?
> > I'm using the latest xen-unstable 19740, Dom0 898, ioemu
> > e0bb6b8df60863bca0163a1688baf4854e931e55.
> 
> Hi Dexuan,
> 
> I think that a simple solution to this is to just remove the
> first invocation.
> 
> -----------------------------------------------------------------------
> 
> xend: pass-through: Only call setupOneDevice() once per device
> 
> As observed by Dexuan Cui, when PCI devices are passed through at
> domain-creation-time setupOneDevice() will be called twice.
> 
> Once via setupDevice() and once via econfigureDevice() which
> is called in pci_device_configure().
> 
> This patch removes the first of these.
> 
> Cc: Dexuan Cui <dexuan.cui@intel.com>
> Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
> ===================================================================
> --- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:00.000000000 +1000
> +++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:02.000000000 +1000
> @@ -436,8 +436,6 @@ class PciController(DevController):
>                                      ' same guest with %s'
>                                  raise VmError(err_msg % (s, dev.name))
>  
> -        for (domain, bus, slot, func) in pci_dev_list:
> -            self.setupOneDevice(domain, bus, slot, func)
>          wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
>          self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
>          log.debug('pci: register aer watch %s', wPath)


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

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

* RE: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-07-28  7:15           ` Simon Horman
@ 2009-07-28  8:42             ` Cui, Dexuan
  2009-07-28  8:56               ` Simon Horman
  0 siblings, 1 reply; 26+ messages in thread
From: Cui, Dexuan @ 2009-07-28  8:42 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel, Keir Fraser

Duplicate calls to setupOneDevice() in the case of HVM  are really odd to me. 

BTW, actually I suspect setupOneDevice() is unnecessary for HVM guest since looks all of the actual work of setupOneDevice() is done by ioemu-dm.

Thanks,
-- Dexuan

-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?7?28? 15:16
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

Hi,

is an alternative to just revert 19754 and allow
duplicate calls to setupOneDevice() in the HVM case?

On Tue, Jul 28, 2009 at 02:47:16PM +0800, Cui, Dexuan wrote:
> Hi Simon, 
> > I think that a simple solution to this is to just remove the first invocation.
> This was checked in as c/s 19754.
> Unluckily, this breaks device assignment for pv guest: xend would not invoke setupOneDevice() for pv guest at all.
> 
> The attached patch fixes the issue.  Please have a look.
> 
> Thanks,
> -- Dexuan
> 
> 
> 
> -----Original Message-----
> From: Simon Horman [mailto:horms@verge.net.au] 
> Sent: 2009?6?15? 9:53
> To: Cui, Dexuan
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
> 
> On Fri, Jun 12, 2009 at 02:35:02PM +0800, Cui, Dexuan wrote:
> > On Fri, Jun 12, 2009 at 14:34, Simon Horman wrote:
> > > On Fri, Jun 12, 2009 at 01:51:10PM +0800, Cui, Dexuan wrote:
> > > > Hi Simon,
> > > > After this changeset, I find there are some new issues in the xend:
> > > > I noticed in xend.log, setupOneDevice() is invoked twice,
> > > > but actually I only statically assign 1 device to hvm guest.
> > > > 
> > > > After looking into the xend code, I find in XendDomainInfo.py:
> > > > _initDomain() -> _createDevices(), we invoke
> > > > self._createDevice(devclass, config) that eventually invokes
> > > > setupOneDevice() -- this is the first time;
> > > > And later, still in  _createDevices(), we invoke
> > > > pci_device_configure_boot() -> pci_device_configure() ->
> > > > dev_control.reconfigureDevice(devid, dev_config) ->
> > > > xend/server/pciif.py:reconfigureDevice() -> setupOneDevice()
> > > > --  this is the second time.  Can you remove the duplicate invocation?
> > > 
> > > Sure, I will look into it ASAP.
> > 
> > > Can I confirm which version of xen-unstable.hg and qemu-xen-unstable.git
> > > you are using?
> > I'm using the latest xen-unstable 19740, Dom0 898, ioemu
> > e0bb6b8df60863bca0163a1688baf4854e931e55.
> 
> Hi Dexuan,
> 
> I think that a simple solution to this is to just remove the
> first invocation.
> 
> -----------------------------------------------------------------------
> 
> xend: pass-through: Only call setupOneDevice() once per device
> 
> As observed by Dexuan Cui, when PCI devices are passed through at
> domain-creation-time setupOneDevice() will be called twice.
> 
> Once via setupDevice() and once via econfigureDevice() which
> is called in pci_device_configure().
> 
> This patch removes the first of these.
> 
> Cc: Dexuan Cui <dexuan.cui@intel.com>
> Cc: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> Index: xen-unstable.hg/tools/python/xen/xend/server/pciif.py
> ===================================================================
> --- xen-unstable.hg.orig/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:00.000000000 +1000
> +++ xen-unstable.hg/tools/python/xen/xend/server/pciif.py	2009-06-15 11:24:02.000000000 +1000
> @@ -436,8 +436,6 @@ class PciController(DevController):
>                                      ' same guest with %s'
>                                  raise VmError(err_msg % (s, dev.name))
>  
> -        for (domain, bus, slot, func) in pci_dev_list:
> -            self.setupOneDevice(domain, bus, slot, func)
>          wPath = '/local/domain/0/backend/pci/%u/0/aerState' % (self.getDomid())
>          self.aerStateWatch = xswatch(wPath, self._handleAerStateWatch)
>          log.debug('pci: register aer watch %s', wPath)


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

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

* Re: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-07-28  8:42             ` Cui, Dexuan
@ 2009-07-28  8:56               ` Simon Horman
  2009-07-28  9:04                 ` Cui, Dexuan
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Horman @ 2009-07-28  8:56 UTC (permalink / raw)
  To: Cui, Dexuan; +Cc: xen-devel, Keir Fraser

On Tue, Jul 28, 2009 at 04:42:17PM +0800, Cui, Dexuan wrote:
> Duplicate calls to setupOneDevice() in the case of HVM  are really odd to me. 

Personally I'd rather call the code twice than have extra if logic.
But your patch is ok with me.

> BTW, actually I suspect setupOneDevice() is unnecessary for HVM guest
> since looks all of the actual work of setupOneDevice() is done by
> ioemu-dm.

I suspect you are right there.

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

* RE: RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time
  2009-07-28  8:56               ` Simon Horman
@ 2009-07-28  9:04                 ` Cui, Dexuan
  0 siblings, 0 replies; 26+ messages in thread
From: Cui, Dexuan @ 2009-07-28  9:04 UTC (permalink / raw)
  To: Simon Horman; +Cc: xen-devel, Keir Fraser

Hi Simon,
Actually I think you have done a great work to clean up the related xend code, however looks we should do more. :-)

Thanks,
-- Dexuan


-----Original Message-----
From: Simon Horman [mailto:horms@verge.net.au] 
Sent: 2009?7?28? 16:57
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time

On Tue, Jul 28, 2009 at 04:42:17PM +0800, Cui, Dexuan wrote:
> Duplicate calls to setupOneDevice() in the case of HVM  are really odd to me. 

Personally I'd rather call the code twice than have extra if logic.
But your patch is ok with me.

> BTW, actually I suspect setupOneDevice() is unnecessary for HVM guest
> since looks all of the actual work of setupOneDevice() is done by
> ioemu-dm.

I suspect you are right there.

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

end of thread, other threads:[~2009-07-28  9:04 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200905300930.n4U9UOTG008828@xenbits.xensource.com>
2009-06-02  5:05 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
2009-06-02  5:16   ` Simon Horman
2009-06-02  8:38     ` Cui, Dexuan
2009-06-02  9:51       ` Keir Fraser
2009-06-02 10:02         ` Ian Jackson
2009-06-02 11:18       ` Simon Horman
2009-06-02 11:23         ` Cui, Dexuan
2009-06-02 11:31         ` Cui, Dexuan
2009-06-02 12:52           ` Simon Horman
2009-06-02 15:21             ` Simon Horman
2009-06-03  2:50               ` Simon Horman
2009-06-03  5:40                 ` Cui, Dexuan
2009-06-03  6:04                   ` Simon Horman
2009-06-03  3:09               ` Cui, Dexuan
2009-06-02 21:07       ` Information about xm list! Ata Bohra
2009-06-12  5:51 ` [Xen-changelog] [xen-unstable] xend: hot-plug PCI devices at boot-time Cui, Dexuan
2009-06-12  6:33   ` Simon Horman
2009-06-12  6:35     ` Cui, Dexuan
2009-06-12  6:45       ` Simon Horman
2009-06-15  1:53       ` Simon Horman
2009-06-15  2:01         ` Cui, Dexuan
2009-07-28  6:47         ` Cui, Dexuan
2009-07-28  7:15           ` Simon Horman
2009-07-28  8:42             ` Cui, Dexuan
2009-07-28  8:56               ` Simon Horman
2009-07-28  9:04                 ` Cui, Dexuan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.