[PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[cyliu]

One of our customers requests parallel pci passthrough functionality between xen
(xend and libxl) and kvm, including support managed host pci devices. A 
"managed" pci device will be made assignable before vm start  and reattach to 
its original dirver after vm shut off.

Currently, libvirt supports "managed=yes/no" options in pci device definition.
Qemu driver already supports managed pci devices, libxl driver will add that
support in libvirt source code. For xend driver, since it's stateful, libvirt
can't do much things because libvirt doesn't store much informtion and most
work is done by calling xend directly. Even "managed" option won't be stored if
xend doesn't support it. For that reason, this patch series tries to add code in
xend toolstack to support managed pci devices first, then libvirt can call xend
operations directly to support "managed" host pci devices. 

Syntax for managed pci device could be:
pci=['0000:00:1a.0,managed=1']

Please share your comments. Thanks!

[PATCH 1 of 2] pci passtrough: add xm pci-assignable-add/remove commands

[cyliu]

Add two commands to xm, make xend toolstack parallel to xl toolstack. Functions
can be used in managed pci devices handling.

Signed-off-by: Chunyan Liu <cyliu@suse.com>

diff -r 64b36dde26bc -r f0b2ddf43585 tools/python/xen/util/pci.py
--- a/tools/python/xen/util/pci.py	Fri Jan 04 15:58:37 2013 +0000
+++ b/tools/python/xen/util/pci.py	Tue Jan 08 15:03:17 2013 +0800
@@ -20,6 +20,7 @@ from xen.xend import sxp
 from xen.xend.XendConstants import AUTO_PHP_SLOT
 from xen.xend.XendSXPDev import dev_dict_to_sxp
 from xen.xend.XendLogging import log
+from xen.xend.xenstore.xstransact import xstransact
 
 # for 2.3 compatibility
 try:
@@ -27,9 +28,11 @@ try:
 except NameError:
     from sets import Set as set
 
+XS_PCIBACK_PATH = '/xm/pciback'
 PROC_PCI_PATH = '/proc/bus/pci/devices'
 PROC_PCI_NUM_RESOURCES = 7
 
+SYSFS_PCI_DRVS_PATH = 'bus/pci/drivers'
 SYSFS_PCI_DEVS_PATH = '/bus/pci/devices'
 SYSFS_PCI_DEV_RESOURCE_PATH = '/resource'
 SYSFS_PCI_DEV_CONFIG_PATH = '/config'
@@ -427,6 +430,9 @@ def __pci_dict_to_fmt_str(fmt, dev):
 def pci_dict_to_bdf_str(dev):
     return __pci_dict_to_fmt_str('%04x:%02x:%02x.%01x', dev)
 
+def pci_dict_to_xs_bdf_str(dev):
+    return __pci_dict_to_fmt_str('%04x-%02x-%02x-%01x', dev)
+
 def pci_dict_to_xc_str(dev):
     return __pci_dict_to_fmt_str('0x%x, 0x%x, 0x%x, 0x%x', dev)
 
@@ -560,6 +566,111 @@ def find_all_assignable_devices():
         dev_list = dev_list + [dev]
     return dev_list
 
+def pci_assignable_add(dev, rebind):
+    '''detach pci device from driver that we need to unbind from and rebind
+       to pciback driver, then it can be assigned to guest.
+    '''
+    sysfs_mnt = find_sysfs_mnt()
+    pcidev_path = sysfs_mnt + SYSFS_PCI_DEVS_PATH
+    pciback_path = sysfs_mnt + SYSFS_PCIBACK_PATH
+
+    # See if the device exists
+    pci_bdf = pci_dict_to_bdf_str(dev)
+    path = pcidev_path + '/' + pci_bdf
+    if not os.path.exists(path):
+        log.debug("Pci device %s doesn't exist" % pci_bdf)
+        return -1
+
+    # Check to see if it's already assigned to pciback
+    path = pciback_path + '/' + pci_bdf
+    if os.path.exists(path):
+        log.debug("Pci device %s is already assigned to pciback" % pci_bdf)
+        return 0
+
+    # Check to see if there's already a driver that we need to unbind from
+    path = pcidev_path + '/' + pci_bdf + '/driver'
+    drv_path = None
+    if os.path.exists(path):
+        drv_path = os.path.realpath(path)
+        cmd = 'echo %s > %s/unbind' % (pci_bdf, drv_path)
+        if os.system(cmd):
+            log.debug("Couldn't unbind device")
+            return -1;
+
+    # Store Store driver_path for rebinding to dom0
+    if rebind:
+        if drv_path is not None: 
+            xs_pci_bdf = pci_dict_to_xs_bdf_str(dev)
+            path = XS_PCIBACK_PATH + '/' + xs_pci_bdf
+            xstransact.Mkdir(path)
+            xstransact.Write(path, 'driver_path', drv_path)
+        else:
+            log.debug("Not bound to a driver, will not be rebound")
+
+    # Bind to pciback
+    # Scan through /sys/.../pciback/slots looking for pcidev's BDF
+    slots = os.popen('cat %s/slots' % pciback_path).read()
+    if re.search(pci_bdf, slots) is None:
+        # write bdf to new_slot
+        cmd = 'echo %s > %s/new_slot' % (pci_bdf, pciback_path)
+        if os.system(cmd):
+            log.debug("Couldn't add device to pciback new_slot")
+            return -1
+
+    # Bind to pciback
+    cmd = 'echo %s > %s/bind' % (pci_bdf, pciback_path)
+    if os.system(cmd):
+        log.debug("Couldn't bind device to pciback")
+        return -1
+
+    return 0
+
+def pci_assignable_remove(dev, rebind):
+    '''unbind pci device from pciback, and rebind to host pci driver where it
+       was detached from in pci-assignable-add.
+    '''
+    sysfs_mnt = find_sysfs_mnt()
+    pcidrv_path = sysfs_mnt + SYSFS_PCI_DRVS_PATH
+    pciback_path = sysfs_mnt + SYSFS_PCIBACK_PATH
+    pci_bdf = pci_dict_to_bdf_str(dev)
+
+    # Unbind from pciback
+    path = pciback_path + '/' + pci_bdf
+    if os.path.exists(path):
+        # unbind
+        cmd = 'echo %s > %s/unbind' % (pci_bdf, pciback_path)
+        if os.system(cmd):
+            log.debug("Couldn't unbind device to pciback")
+            return -1
+
+        # remove slots if necessary
+        slots = os.popen('cat %s/slots' % pciback_path).read()
+        if re.search(pci_bdf, slots):
+            # write bdf to remove_slot
+            cmd = 'echo %s > %s/remove_slot' % (pci_bdf, pciback_path)
+            if os.system(cmd):
+                log.debug("Couldn't remove pciback slot")
+                return -1
+    else:
+        log.debug("Not bound to pciback")
+
+    # Rebind if necessary
+    xs_pci_bdf = pci_dict_to_xs_bdf_str(dev)
+    path = XS_PCIBACK_PATH + '/' + xs_pci_bdf
+    drv_path = xstransact.Read(path, 'driver_path')
+    if drv_path:
+        if rebind:
+            cmd = 'echo %s > %s/bind' % (pci_bdf, drv_path)
+            if os.system(cmd):
+                log.debug("Couldn't rebind to driver %s" % drv_path)
+                return -1
+        xstransact.Remove(path)
+    else:
+        if rebind:
+            log.debug("Counldn't find path for original driver.Not rebinding")
+
+    return 0
+
 def transform_list(target, src):
     ''' src: its element is pci string (Format: xxxx:xx:xx.x).
         target: its element is pci string, or a list of pci string.
diff -r 64b36dde26bc -r f0b2ddf43585 tools/python/xen/xm/main.py
--- a/tools/python/xen/xm/main.py	Fri Jan 04 15:58:37 2013 +0000
+++ b/tools/python/xen/xm/main.py	Tue Jan 08 15:03:17 2013 +0800
@@ -211,6 +211,10 @@ SUBCOMMAND_HELP = {
     'pci-list'      :  ('<Domain>',
                         'List pass-through pci devices for a domain.'),
     'pci-list-assignable-devices' : ('', 'List all the assignable pci devices'),
+    'pci-assignable-add' : ('<domain:bus:slot.func>',
+                            'Add a pci device to assignable list.'),
+    'pci-assignable-remove' : ('<domain:bus:slot.func>',
+                               'Remove pci device from assignable list.'),
     'scsi-attach'  :  ('<Domain> <PhysDevice> <VirtDevice> [BackDomain]',
                         'Attach a new SCSI device.'),
     'scsi-detach'  :  ('<Domain> <VirtDevice>',
@@ -472,6 +476,8 @@ device_commands = [
     "pci-detach",
     "pci-list",
     "pci-list-assignable-devices",
+    "pci-assignable-add",
+    "pci-assignable-remove",
     "scsi-attach",
     "scsi-detach",
     "scsi-list",
@@ -2488,6 +2494,21 @@ def xm_pci_list_assignable_devices(args)
         pci = parse_pci_info(x)
         print fmt_str % pci
 
+def xm_pci_assignable_add(args):
+    xenapi_unsupported()
+    arg_check(args, "pci-assignable-add", 1)
+
+    pci_dev_str = args[0]
+    pci_dev = parse_pci_name(pci_dev_str)
+    pci_assignable_add(pci_dev, 1)
+
+def xm_pci_assignable_remove(args):
+    xenapi_unsupported()
+    arg_check(args, "pci-assignable-remove", 1)
+    
+    pci_dev_str = args[0]
+    pci_dev = parse_pci_name(pci_dev_str)
+    pci_assignable_remove(pci_dev, 1)
 
 def vscsi_sort(devs):
     def sort_hctl(ds, l):
@@ -3824,6 +3845,8 @@ commands = {
     "pci-detach": xm_pci_detach,
     "pci-list": xm_pci_list,
     "pci-list-assignable-devices": xm_pci_list_assignable_devices,
+    "pci-assignable-add": xm_pci_assignable_add,
+    "pci-assignable-remove": xm_pci_assignable_remove,
     # vscsi
     "scsi-attach": xm_scsi_attach,
     "scsi-detach": xm_scsi_detach,

[PATCH 2 of 2] pci passthrough: handle managed pci devices

[cyliu]

Handle managed pci devices for libvirt usage. If a pci device is set
"managed=1", it will be made assignable (unbound from original driver and bind
to pcistub driver) before vm start and reattach to original driver after vm
shut off.

Signed-off-by: Chunyan Liu <cyliu@suse.com>

diff -r f0b2ddf43585 -r 930a9671951d tools/python/xen/util/pci.py
--- a/tools/python/xen/util/pci.py	Tue Jan 08 15:03:17 2013 +0800
+++ b/tools/python/xen/util/pci.py	Wed Jan 09 11:46:22 2013 +0800
@@ -164,7 +164,7 @@ def PCI_BDF(domain, bus, slot, func):
 
 def check_pci_opts(opts):
     def f((k, v)):
-        if k not in ['msitranslate', 'power_mgmt'] or \
+        if k not in ['msitranslate', 'power_mgmt', 'managed'] or \
            not v.lower() in ['0', '1', 'yes', 'no']:
             raise PciDeviceParseError('Invalid pci option %s=%s: ' % (k, v))
 
diff -r f0b2ddf43585 -r 930a9671951d tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Tue Jan 08 15:03:17 2013 +0800
+++ b/tools/python/xen/xend/XendDomainInfo.py	Wed Jan 09 11:46:22 2013 +0800
@@ -303,7 +303,8 @@ def dom_get(dom):
     return None
 
 from xen.xend.server.pciif import parse_pci_name, PciDevice,\
-    get_assigned_pci_devices, get_all_assigned_pci_devices
+    get_assigned_pci_devices, get_all_assigned_pci_devices,\
+    prepare_host_pci_devices, reattach_host_pci_devices
 
 
 def do_FLR(domid, is_hvm):
@@ -317,6 +318,20 @@ def do_FLR(domid, is_hvm):
                     "parse it's resources - "+str(e))
         dev.do_FLR(is_hvm, xoptions.get_pci_dev_assign_strict_check())
 
+def prepare_domain_pci_devices(domconfig):
+    ordered_refs = domconfig.ordered_device_refs()
+    for dev_uuid in ordered_refs:
+        devclass, devconfig = domconfig['devices'][dev_uuid]
+        if devclass == 'pci':
+            prepare_host_pci_devices(devconfig)
+
+def reattach_domain_pci_devices(domconfig):
+    ordered_refs = domconfig.ordered_device_refs()
+    for dev_uuid in ordered_refs:
+        devclass, devconfig = domconfig['devices'][dev_uuid]
+        if devclass == 'pci':
+            reattach_host_pci_devices(devconfig)
+
 class XendDomainInfo:
     """An object represents a domain.
 
@@ -470,6 +485,7 @@ class XendDomainInfo:
 
         if self._stateGet() in (XEN_API_VM_POWER_STATE_HALTED, XEN_API_VM_POWER_STATE_SUSPENDED, XEN_API_VM_POWER_STATE_CRASHED):
             try:
+                prepare_domain_pci_devices(self.info);
                 XendTask.log_progress(0, 30, self._constructDomain)
                 XendTask.log_progress(31, 60, self._initDomain)
                 
@@ -496,6 +512,7 @@ class XendDomainInfo:
         state = self._stateGet()
         if state in (DOM_STATE_SUSPENDED, DOM_STATE_HALTED):
             try:
+                prepare_domain_pci_devices(self.info)
                 self._constructDomain()
 
                 try:
@@ -838,6 +855,9 @@ class XendDomainInfo:
         log.debug("XendDomainInfo.device_create: %s" % scrub_password(dev_config))
         dev_type = sxp.name(dev_config)
 
+        if dev_type == 'pci':
+            prepare_host_pci_devices(devconfig)
+
         if dev_type == 'vif':
             for x in dev_config:
                 if x != 'vif' and x[0] == 'mac':
@@ -3099,6 +3119,7 @@ class XendDomainInfo:
                 log.debug("%s KiB need to add to Memory pool" %self.alloc_mem)
                 MemoryPool.instance().increase_memory(self.alloc_mem)
 
+        reattach_domain_pci_devices(self.info)
         self._cleanup_phantom_devs(paths)
         self._cleanupVm()
 
diff -r f0b2ddf43585 -r 930a9671951d tools/python/xen/xend/server/pciif.py
--- a/tools/python/xen/xend/server/pciif.py	Tue Jan 08 15:03:17 2013 +0800
+++ b/tools/python/xen/xend/server/pciif.py	Wed Jan 09 11:46:22 2013 +0800
@@ -86,6 +86,32 @@ def get_all_assigned_pci_devices(domid =
             pci_str_list = pci_str_list + get_assigned_pci_devices(int(d))
     return pci_str_list
 
+def prepare_host_pci_devices(devconfig):
+    pci_dev_list = devconfig.get('devs', [])
+    for pci_dev in pci_dev_list:
+        managed = 0
+        pci_opts_config = pci_dev.get('opts', [])
+        for opt in pci_opts_config:
+            if opt[0] == 'managed':
+                managed = opt[1]
+        if managed:
+            if pci_assignable_add(pci_dev, 1) != 0:
+                raise VmError('pci_assignable_add failed')
+    return
+
+def reattach_host_pci_devices(devconfig):
+    pci_dev_list = devconfig.get('devs', [])
+    for pci_dev in pci_dev_list:
+        managed = 0
+        pci_opts_config = pci_dev['opts']
+        for opt in pci_opts_config:
+            if opt[0] == 'managed':
+                managed = opt[1]
+        if managed:
+            if pci_assignable_remove(pci_dev, 1) != 0:
+               raise VmError('pci_assignable_remove failed')
+    return
+
 class PciController(DevController):
 
     def __init__(self, vm):
diff -r f0b2ddf43585 -r 930a9671951d tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py	Tue Jan 08 15:03:17 2013 +0800
+++ b/tools/python/xen/xm/create.py	Wed Jan 09 11:46:22 2013 +0800
@@ -332,7 +332,7 @@ gopts.var('disk', val='phy:DEV,VDEV,MODE
           backend driver domain to use for the disk.
           The option may be repeated to add more than one disk.""")
 
-gopts.var('pci', val='BUS:DEV.FUNC[@VSLOT][,msitranslate=0|1][,power_mgmt=0|1]',
+gopts.var('pci', val='BUS:DEV.FUNC[@VSLOT][,msitranslate=0|1][,power_mgmt=0|1][,managed=0|1]',
           fn=append_value, default=[],
           use="""Add a PCI device to a domain, using given params (in hex).
           For example 'pci=c0:02.1'.
@@ -343,7 +343,9 @@ gopts.var('pci', val='BUS:DEV.FUNC[@VSLO
           translated from physical MSI, HVM only. Default is 1.
           The option may be repeated to add more than one pci device.
           If power_mgmt is set, the guest OS will be able to program the power
-          states D0-D3hot of the device, HVM only. Default=0.""")
+          states D0-D3hot of the device, HVM only. Default=0.
+          If managed is set, pci device will be prepared and released when VM
+          is started or shut off""")
 
 gopts.var('vscsi', val='PDEV,VDEV[,DOM]',
           fn=append_value, default=[],

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[George Dunlap]

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

On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com> wrote:

> One of our customers requests parallel pci passthrough functionality
> between xen
> (xend and libxl) and kvm, including support managed host pci devices. A
> "managed" pci device will be made assignable before vm start  and reattach
> to
> its original dirver after vm shut off.
>
> Currently, libvirt supports "managed=yes/no" options in pci device
> definition.
> Qemu driver already supports managed pci devices, libxl driver will add
> that
> support in libvirt source code. For xend driver, since it's stateful,
> libvirt
> can't do much things because libvirt doesn't store much informtion and most
> work is done by calling xend directly. Even "managed" option won't be
> stored if
> xend doesn't support it. For that reason, this patch series tries to add
> code in
> xend toolstack to support managed pci devices first, then libvirt can call
> xend
> operations directly to support "managed" host pci devices.
>
> Syntax for managed pci device could be:
> pci=['0000:00:1a.0,managed=1']
>
> Please share your comments. Thanks!
>

The first question (before I look at the code closely) is whether we want
to accept new features into xend.  It's not being actively maintained, and
we would like to get rid of it at some point.

Given that you seem primarily to be using libvirt, after the 4.3 release,
will there be a strong reason to use xend, instead of just using libxl?

Note I'm not rejecting it outright; I just think a case needs to be made.
:-)

 -George

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

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

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

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[Jim Fehlig]

George Dunlap wrote:
> On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com
> <mailto:cyliu@suse.com>> wrote:
>
>     One of our customers requests parallel pci passthrough
>     functionality between xen
>     (xend and libxl) and kvm, including support managed host pci
>     devices. A
>     "managed" pci device will be made assignable before vm start  and
>     reattach to
>     its original dirver after vm shut off.
>
>     Currently, libvirt supports "managed=yes/no" options in pci device
>     definition.
>     Qemu driver already supports managed pci devices, libxl driver
>     will add that
>     support in libvirt source code. For xend driver, since it's
>     stateful, libvirt
>     can't do much things because libvirt doesn't store much informtion
>     and most
>     work is done by calling xend directly. Even "managed" option won't
>     be stored if
>     xend doesn't support it. For that reason, this patch series tries
>     to add code in
>     xend toolstack to support managed pci devices first, then libvirt
>     can call xend
>     operations directly to support "managed" host pci devices.
>
>     Syntax for managed pci device could be:
>     pci=['0000:00:1a.0,managed=1']
>
>     Please share your comments. Thanks!
>
>
> The first question (before I look at the code closely) is whether we
> want to accept new features into xend.  It's not being actively
> maintained, and we would like to get rid of it at some point.
>
> Given that you seem primarily to be using libvirt, after the 4.3
> release, will there be a strong reason to use xend, instead of just
> using libxl?

Our SLE11 enterprise product uses the legacy toolstack and I doubt we
will change that until SLE12.  We need to give users time to migrate
from the old toolstack as well.

Chunyan first added this functionality to the libvirt libxl driver [1],
since it is preferred going forward.  Unfortunately we need to provide
the same functionality in the old toolstack.  We can carry this patch in
our packages if needed, but upstream backports are certainly preferred
over local patches.

> Note I'm not rejecting it outright; I just think a case needs to be
> made. :-)

Ok, I've made an attempt :).

Regards,
Jim

[1] https://www.redhat.com/archives/libvir-list/2013-January/msg00689.html

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[George Dunlap]

On 17/01/13 19:12, Jim Fehlig wrote:
> George Dunlap wrote:
>> On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com
>> <mailto:cyliu@suse.com>> wrote:
>>
>>      One of our customers requests parallel pci passthrough
>>      functionality between xen
>>      (xend and libxl) and kvm, including support managed host pci
>>      devices. A
>>      "managed" pci device will be made assignable before vm start  and
>>      reattach to
>>      its original dirver after vm shut off.
>>
>>      Currently, libvirt supports "managed=yes/no" options in pci device
>>      definition.
>>      Qemu driver already supports managed pci devices, libxl driver
>>      will add that
>>      support in libvirt source code. For xend driver, since it's
>>      stateful, libvirt
>>      can't do much things because libvirt doesn't store much informtion
>>      and most
>>      work is done by calling xend directly. Even "managed" option won't
>>      be stored if
>>      xend doesn't support it. For that reason, this patch series tries
>>      to add code in
>>      xend toolstack to support managed pci devices first, then libvirt
>>      can call xend
>>      operations directly to support "managed" host pci devices.
>>
>>      Syntax for managed pci device could be:
>>      pci=['0000:00:1a.0,managed=1']
>>
>>      Please share your comments. Thanks!
>>
>>
>> The first question (before I look at the code closely) is whether we
>> want to accept new features into xend.  It's not being actively
>> maintained, and we would like to get rid of it at some point.
>>
>> Given that you seem primarily to be using libvirt, after the 4.3
>> release, will there be a strong reason to use xend, instead of just
>> using libxl?
>
> Our SLE11 enterprise product uses the legacy toolstack and I doubt we
> will change that until SLE12.  We need to give users time to migrate
> from the old toolstack as well.
>
> Chunyan first added this functionality to the libvirt libxl driver [1],
> since it is preferred going forward.  Unfortunately we need to provide
> the same functionality in the old toolstack.  We can carry this patch in
> our packages if needed, but upstream backports are certainly preferred
> over local patches.

So I'm hearing that one reason you want it upstream is because you 
prefer to have a backport, rather than just having a stand-alone patch 
in your queue.

That's a very good general policy, but it's not necessarily a reason why 
xen.org should take the patch.  The main reason we would take the patch 
would be, "SuSE will use it in 4.3".

But it's not clear that's the case -- are you planning on pulling Xen 
4.3 into SLE 11?  Do you think that you'll need xend in SLE12 "to give 
users time to migrate"?

If we really are going to get rid of xend, there must be a point where 
users are "pushed", by lack of features (or lack of existence) onto the 
new toolstack.  Feature parity in new releases is only going to delay 
the inevitable.

We've tried to make that step as simple as possible, by making xl 
compatible with xend, and by making sure key functionality has been 
carried over.  If there are still things that will make that transition 
hard, maybe you could point those out and we can see if we can address them?

Overall it seems like if we stick with straight principles, we shouldn't 
take the patch.

But I'm not adamant -- I'd be interested in hearing other opinions.

The other option, of course, would be for someone / some organization to 
commit to being the xend maintainer going forward -- which would 
probably involve committing to porting new libxl features over to xend. 
  I don't think that's recommended, but everyone can spend their own 
money / engineering hours how they like. :-)

  -George

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[Konrad Rzeszutek Wilk]

On Thu, Jan 17, 2013 at 06:00:56PM +0000, George Dunlap wrote:
> On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com> wrote:
> 
> > One of our customers requests parallel pci passthrough functionality
> > between xen
> > (xend and libxl) and kvm, including support managed host pci devices. A
> > "managed" pci device will be made assignable before vm start  and reattach
> > to
> > its original dirver after vm shut off.
> >
> > Currently, libvirt supports "managed=yes/no" options in pci device
> > definition.
> > Qemu driver already supports managed pci devices, libxl driver will add
> > that
> > support in libvirt source code. For xend driver, since it's stateful,
> > libvirt
> > can't do much things because libvirt doesn't store much informtion and most
> > work is done by calling xend directly. Even "managed" option won't be
> > stored if
> > xend doesn't support it. For that reason, this patch series tries to add
> > code in
> > xend toolstack to support managed pci devices first, then libvirt can call
> > xend
> > operations directly to support "managed" host pci devices.
> >
> > Syntax for managed pci device could be:
> > pci=['0000:00:1a.0,managed=1']
> >
> > Please share your comments. Thanks!
> >
> 
> The first question (before I look at the code closely) is whether we want
> to accept new features into xend.  It's not being actively maintained, and
> we would like to get rid of it at some point.
> 
> Given that you seem primarily to be using libvirt, after the 4.3 release,
> will there be a strong reason to use xend, instead of just using libxl?
> 
> Note I'm not rejecting it outright; I just think a case needs to be made.

I am actually in favour of adding things in both. There are multiple distros
that are still using Xen 4.1 and some of them have a long life-cycle. Adding in
features that are also reflected in 'xl' looks to be the doing the right
thing for the customers and with the future in mind.

> :-)
> 
>  -George

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

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[Ian Campbell]

On Fri, 2013-01-18 at 15:48 +0000, Konrad Rzeszutek Wilk wrote:
> I am actually in favour of adding things in both. There are multiple
> distros that are still using Xen 4.1 and some of them have a long
> life-cycle.

Even if we were to take xend patches in unstable I wouldn't expect them
to be backported to 4.1 at this point.

>  Adding in features that are also reflected in 'xl' looks to be the
> doing the right thing for the customers and with the future in mind.

In order for us to start habitually taking patches which implement new
features in xend again someone needs to step up and take responsibility
for it. i.e. reviewing the patches and fixing the regressions they
introduce. Probably they should add themselves to MAINTAINERS.

I've no objections to someone who wants to become the xend maintainer
stepping up and doing the job but just slapping patches in isn't the
answer.

Personally I think that having made the decision to deprecate xend we
should just let it die and not prolong the pain.

Ian.

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[Jim Fehlig]

George Dunlap wrote:
> On 17/01/13 19:12, Jim Fehlig wrote:
>> George Dunlap wrote:
>>> On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com
>>> <mailto:cyliu@suse.com>> wrote:
>>>
>>>      One of our customers requests parallel pci passthrough
>>>      functionality between xen
>>>      (xend and libxl) and kvm, including support managed host pci
>>>      devices. A
>>>      "managed" pci device will be made assignable before vm start  and
>>>      reattach to
>>>      its original dirver after vm shut off.
>>>
>>>      Currently, libvirt supports "managed=yes/no" options in pci device
>>>      definition.
>>>      Qemu driver already supports managed pci devices, libxl driver
>>>      will add that
>>>      support in libvirt source code. For xend driver, since it's
>>>      stateful, libvirt
>>>      can't do much things because libvirt doesn't store much informtion
>>>      and most
>>>      work is done by calling xend directly. Even "managed" option won't
>>>      be stored if
>>>      xend doesn't support it. For that reason, this patch series tries
>>>      to add code in
>>>      xend toolstack to support managed pci devices first, then libvirt
>>>      can call xend
>>>      operations directly to support "managed" host pci devices.
>>>
>>>      Syntax for managed pci device could be:
>>>      pci=['0000:00:1a.0,managed=1']
>>>
>>>      Please share your comments. Thanks!
>>>
>>>
>>> The first question (before I look at the code closely) is whether we
>>> want to accept new features into xend.  It's not being actively
>>> maintained, and we would like to get rid of it at some point.
>>>
>>> Given that you seem primarily to be using libvirt, after the 4.3
>>> release, will there be a strong reason to use xend, instead of just
>>> using libxl?
>>
>> Our SLE11 enterprise product uses the legacy toolstack and I doubt we
>> will change that until SLE12.  We need to give users time to migrate
>> from the old toolstack as well.
>>
>> Chunyan first added this functionality to the libvirt libxl driver [1],
>> since it is preferred going forward.  Unfortunately we need to provide
>> the same functionality in the old toolstack.  We can carry this patch in
>> our packages if needed, but upstream backports are certainly preferred
>> over local patches.
>
> So I'm hearing that one reason you want it upstream is because you
> prefer to have a backport, rather than just having a stand-alone patch
> in your queue.
>
> That's a very good general policy, but it's not necessarily a reason
> why xen.org should take the patch.  The main reason we would take the
> patch would be, "SuSE will use it in 4.3".
>
> But it's not clear that's the case -- are you planning on pulling Xen
> 4.3 into SLE 11?

Probably not.

>   Do you think that you'll need xend in SLE12 "to give users time to
> migrate"?

No.

> If we really are going to get rid of xend, there must be a point where
> users are "pushed", by lack of features (or lack of existence) onto
> the new toolstack.  Feature parity in new releases is only going to
> delay the inevitable.
>
> We've tried to make that step as simple as possible, by making xl
> compatible with xend, and by making sure key functionality has been
> carried over.  If there are still things that will make that
> transition hard, maybe you could point those out and we can see if we
> can address them?

I'm not aware of anything.  Users simply need time to migrate their
existing tools, scripts, etc. and we didn't want to force that on them
in a service pack.  A new version of SLE is a different matter.

> Overall it seems like if we stick with straight principles, we
> shouldn't take the patch.
>
> But I'm not adamant -- I'd be interested in hearing other opinions.
>
> The other option, of course, would be for someone / some organization
> to commit to being the xend maintainer going forward -- which would
> probably involve committing to porting new libxl features over to
> xend.  I don't think that's recommended, but everyone can spend their
> own money / engineering hours how they like. :-)

I wouldn't recommend that either, and designating someone as the xend
maintainer is inhumane :).

Jim

Re: [PATCH 0 of 2] pci passthrough: support "managed" pci device in xend for libvirt usage

[George Dunlap]

On 21/01/13 04:50, Jim Fehlig wrote:
> George Dunlap wrote:
>> On 17/01/13 19:12, Jim Fehlig wrote:
>>> George Dunlap wrote:
>>>> On Thu, Jan 17, 2013 at 5:29 AM, <cyliu@suse.com
>>>> <mailto:cyliu@suse.com>> wrote:
>>>>
>>>>       One of our customers requests parallel pci passthrough
>>>>       functionality between xen
>>>>       (xend and libxl) and kvm, including support managed host pci
>>>>       devices. A
>>>>       "managed" pci device will be made assignable before vm start  and
>>>>       reattach to
>>>>       its original dirver after vm shut off.
>>>>
>>>>       Currently, libvirt supports "managed=yes/no" options in pci device
>>>>       definition.
>>>>       Qemu driver already supports managed pci devices, libxl driver
>>>>       will add that
>>>>       support in libvirt source code. For xend driver, since it's
>>>>       stateful, libvirt
>>>>       can't do much things because libvirt doesn't store much informtion
>>>>       and most
>>>>       work is done by calling xend directly. Even "managed" option won't
>>>>       be stored if
>>>>       xend doesn't support it. For that reason, this patch series tries
>>>>       to add code in
>>>>       xend toolstack to support managed pci devices first, then libvirt
>>>>       can call xend
>>>>       operations directly to support "managed" host pci devices.
>>>>
>>>>       Syntax for managed pci device could be:
>>>>       pci=['0000:00:1a.0,managed=1']
>>>>
>>>>       Please share your comments. Thanks!
>>>>
>>>>
>>>> The first question (before I look at the code closely) is whether we
>>>> want to accept new features into xend.  It's not being actively
>>>> maintained, and we would like to get rid of it at some point.
>>>>
>>>> Given that you seem primarily to be using libvirt, after the 4.3
>>>> release, will there be a strong reason to use xend, instead of just
>>>> using libxl?
>>> Our SLE11 enterprise product uses the legacy toolstack and I doubt we
>>> will change that until SLE12.  We need to give users time to migrate
>>> from the old toolstack as well.
>>>
>>> Chunyan first added this functionality to the libvirt libxl driver [1],
>>> since it is preferred going forward.  Unfortunately we need to provide
>>> the same functionality in the old toolstack.  We can carry this patch in
>>> our packages if needed, but upstream backports are certainly preferred
>>> over local patches.
>> So I'm hearing that one reason you want it upstream is because you
>> prefer to have a backport, rather than just having a stand-alone patch
>> in your queue.
>>
>> That's a very good general policy, but it's not necessarily a reason
>> why xen.org should take the patch.  The main reason we would take the
>> patch would be, "SuSE will use it in 4.3".
>>
>> But it's not clear that's the case -- are you planning on pulling Xen
>> 4.3 into SLE 11?
> Probably not.
>
>>    Do you think that you'll need xend in SLE12 "to give users time to
>> migrate"?
> No.

OK, so it sounds like you're not going to need this in 4.3, so we can 
leave it out of xen.org.

>
>> If we really are going to get rid of xend, there must be a point where
>> users are "pushed", by lack of features (or lack of existence) onto
>> the new toolstack.  Feature parity in new releases is only going to
>> delay the inevitable.
>>
>> We've tried to make that step as simple as possible, by making xl
>> compatible with xend, and by making sure key functionality has been
>> carried over.  If there are still things that will make that
>> transition hard, maybe you could point those out and we can see if we
>> can address them?
> I'm not aware of anything.  Users simply need time to migrate their
> existing tools, scripts, etc. and we didn't want to force that on them
> in a service pack.  A new version of SLE is a different matter.

Sure, and as a distro it totally makes sense to carry it as a patch 
until then.

>
>> Overall it seems like if we stick with straight principles, we
>> shouldn't take the patch.
>>
>> But I'm not adamant -- I'd be interested in hearing other opinions.
>>
>> The other option, of course, would be for someone / some organization
>> to commit to being the xend maintainer going forward -- which would
>> probably involve committing to porting new libxl features over to
>> xend.  I don't think that's recommended, but everyone can spend their
>> own money / engineering hours how they like. :-)
> I wouldn't recommend that either, and designating someone as the xend
> maintainer is inhumane :).

Haha -- I'm glad we agree on that. :-)

  -George