Re: [Xen-devel] [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
On Mon, 2015-07-27 at 18:45 +0100, Wei Liu wrote: I missed the fact that next_bdf is used to parsed user supplied strings when reviewing. The user supplied string is a NULL-terminated string separated by comma. User can supply several PCI devices in that string. There is, however, no delimiter for different devices, hence we can't change the syntax of that string. This patch reinstate the original implementation of next_bdf to preserve the original syntax. The last argument for xc_assign_device is always 0. Specifically it returns us to exactly the state in 9b34056cb4ca~1, I believe? Plus an extra 0 flags parameter? Signed-off-by: Wei Liu wei.l...@citrix.com --- Cc: Tiejun Chen tiejun.c...@intel.com Tiejun, are you actually using this python binding? I don't think we have in tree user. If nobody is using it, I propose we remove this binding in next release. I don't have live example of that string. My analysis is based on reverse-engineering of original code. FWIW I've said several times that it is not necessary to plumb new options such as this through the Python bindings, it is sufficient to pass in whatever value means do as you did before. If a user of the Python bindings wants to then plumb in the ability to actually set the option (i.e. there is a use case for it) then that can be done later. Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
On Tue, Jul 28, 2015 at 11:38:29AM +0100, Ian Campbell wrote: On Mon, 2015-07-27 at 18:45 +0100, Wei Liu wrote: I missed the fact that next_bdf is used to parsed user supplied strings when reviewing. The user supplied string is a NULL-terminated string separated by comma. User can supply several PCI devices in that string. There is, however, no delimiter for different devices, hence we can't change the syntax of that string. This patch reinstate the original implementation of next_bdf to preserve the original syntax. The last argument for xc_assign_device is always 0. Specifically it returns us to exactly the state in 9b34056cb4ca~1, I believe? Plus an extra 0 flags parameter? Yes to both questions. Signed-off-by: Wei Liu wei.l...@citrix.com --- Cc: Tiejun Chen tiejun.c...@intel.com Tiejun, are you actually using this python binding? I don't think we have in tree user. If nobody is using it, I propose we remove this binding in next release. I don't have live example of that string. My analysis is based on reverse-engineering of original code. FWIW I've said several times that it is not necessary to plumb new options such as this through the Python bindings, it is sufficient to pass in whatever value means do as you did before. If a user of the Python bindings wants to then plumb in the ability to actually set the option (i.e. there is a use case for it) then that can be done later. Acked-by: Ian Campbell ian.campb...@citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
I missed the fact that next_bdf is used to parsed user supplied
strings when reviewing. The user supplied string is a NULL-terminated
string separated by comma. User can supply several PCI devices in that
string. There is, however, no delimiter for different devices, hence
we can't change the syntax of that string.
This patch reinstate the original implementation of next_bdf to
preserve the original syntax. The last argument for xc_assign_device
is always 0.
Signed-off-by: Wei Liu wei.l...@citrix.com
---
Cc: Tiejun Chen tiejun.c...@intel.com
Tiejun, are you actually using this python binding? I don't think we
have in tree user.
If nobody is using it, I propose we remove this binding in next
release.
I don't have live example of that string. My analysis is based on
reverse-engineering of original code.
---
tools/python/xen/lowlevel/xc/xc.c | 30 ++
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index c8380d1..2c36eb2 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -592,8 +592,7 @@ static int token_value(char *token)
return strtol(token, NULL, 16);
}
-static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func,
-int *flag)
+static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func)
{
char *token;
@@ -608,17 +607,8 @@ static int next_bdf(char **str, int *seg, int *bus, int
*dev, int *func,
*dev = token_value(token);
token = strchr(token, ',') + 1;
*func = token_value(token);
-token = strchr(token, ',') + 1;
-if ( token ) {
-*flag = token_value(token);
-*str = token + 1;
-}
-else
-{
-/* O means we take strict as our default policy. */
-*flag = 0;
-*str = NULL;
-}
+token = strchr(token, ',');
+*str = token ? token + 1 : NULL;
return 1;
}
@@ -630,14 +620,14 @@ static PyObject *pyxc_test_assign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
@@ -663,21 +653,21 @@ static PyObject *pyxc_assign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
sbdf |= (dev 0x1f) 3;
sbdf |= (func 0x7);
-if ( xc_assign_device(self-xc_handle, dom, sbdf, flag) != 0 )
+if ( xc_assign_device(self-xc_handle, dom, sbdf, 0) != 0 )
{
if (errno == ENOSYS)
sbdf = -1;
@@ -696,14 +686,14 @@ static PyObject *pyxc_deassign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
On 27/07/2015 18:45, Wei Liu wrote: I missed the fact that next_bdf is used to parsed user supplied strings when reviewing. The user supplied string is a NULL-terminated string separated by comma. User can supply several PCI devices in that string. There is, however, no delimiter for different devices, hence we can't change the syntax of that string. This patch reinstate the original implementation of next_bdf to preserve the original syntax. The last argument for xc_assign_device is always 0. Signed-off-by: Wei Liu wei.l...@citrix.com --- Cc: Tiejun Chen tiejun.c...@intel.com Tiejun, are you actually using this python binding? I don't think we have in tree user. If nobody is using it, I propose we remove this binding in next release. I don't have live example of that string. My analysis is based on reverse-engineering of original code. XenServer still uses a few methods from the lowlevel xc bindings, but this only extends to debug situations (our version of xen-bugtool) and I am not aware of us using this particular bit of the bindings. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
On 2015/7/28 1:45, Wei Liu wrote:
I missed the fact that next_bdf is used to parsed user supplied
strings when reviewing. The user supplied string is a NULL-terminated
string separated by comma. User can supply several PCI devices in that
string. There is, however, no delimiter for different devices, hence
we can't change the syntax of that string.
This patch reinstate the original implementation of next_bdf to
preserve the original syntax. The last argument for xc_assign_device
is always 0.
Signed-off-by: Wei Liu wei.l...@citrix.com
---
Cc: Tiejun Chen tiejun.c...@intel.com
Tiejun, are you actually using this python binding? I don't think we
This change is just following to RMRR series but currently we don't use
this. So its fine if you think this don't break any other usages.
Thanks
Tiejun
have in tree user.
If nobody is using it, I propose we remove this binding in next
release.
I don't have live example of that string. My analysis is based on
reverse-engineering of original code.
---
tools/python/xen/lowlevel/xc/xc.c | 30 ++
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index c8380d1..2c36eb2 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -592,8 +592,7 @@ static int token_value(char *token)
return strtol(token, NULL, 16);
}
-static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func,
-int *flag)
+static int next_bdf(char **str, int *seg, int *bus, int *dev, int *func)
{
char *token;
@@ -608,17 +607,8 @@ static int next_bdf(char **str, int *seg, int *bus, int
*dev, int *func,
*dev = token_value(token);
token = strchr(token, ',') + 1;
*func = token_value(token);
-token = strchr(token, ',') + 1;
-if ( token ) {
-*flag = token_value(token);
-*str = token + 1;
-}
-else
-{
-/* O means we take strict as our default policy. */
-*flag = 0;
-*str = NULL;
-}
+token = strchr(token, ',');
+*str = token ? token + 1 : NULL;
return 1;
}
@@ -630,14 +620,14 @@ static PyObject *pyxc_test_assign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
@@ -663,21 +653,21 @@ static PyObject *pyxc_assign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
sbdf |= (dev 0x1f) 3;
sbdf |= (func 0x7);
-if ( xc_assign_device(self-xc_handle, dom, sbdf, flag) != 0 )
+if ( xc_assign_device(self-xc_handle, dom, sbdf, 0) != 0 )
{
if (errno == ENOSYS)
sbdf = -1;
@@ -696,14 +686,14 @@ static PyObject *pyxc_deassign_device(XcObject *self,
uint32_t dom;
char *pci_str;
int32_t sbdf = 0;
-int seg, bus, dev, func, flag;
+int seg, bus, dev, func;
static char *kwd_list[] = { domid, pci, NULL };
if ( !PyArg_ParseTupleAndKeywords(args, kwds, is, kwd_list,
dom, pci_str) )
return NULL;
-while ( next_bdf(pci_str, seg, bus, dev, func, flag) )
+while ( next_bdf(pci_str, seg, bus, dev, func) )
{
sbdf = seg 16;
sbdf |= (bus 0xff) 8;
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
