All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Chen, Tiejun" <tiejun.chen@intel.com>
To: Wei Liu <wei.liu2@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf
Date: Tue, 28 Jul 2015 09:06:38 +0800	[thread overview]
Message-ID: <55B6D59E.6010600@intel.com> (raw)
In-Reply-To: <1438019109-31997-8-git-send-email-wei.liu2@citrix.com>

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.liu2@citrix.com>
> ---
> Cc: Tiejun Chen <tiejun.chen@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;
>

  parent reply	other threads:[~2015-07-28  1:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27 17:45 [PATCH for-4.6 v2 0/8] tools: fixes inspired by Coverity scan Wei Liu
2015-07-27 17:45 ` [PATCH for-4.6 v2 1/8] libxl: properly clean up array in libxl_list_cpupool failure path Wei Liu
2015-07-28  8:39   ` Dario Faggioli
2015-07-28 10:27   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 2/8] xl: lockdir should be lockfile in error message Wei Liu
2015-07-28 10:27   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 3/8] xl: call libxl_dominfo_init in main_list Wei Liu
2015-07-28 10:28   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 4/8] xl: valid fd can be 0 in main_loadpolicy Wei Liu
2015-07-28 10:28   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 5/8] xl: call libxl_dominfo_{init, dispose} in main_cpupoolnumasplit Wei Liu
2015-07-28  9:49   ` Dario Faggioli
2015-07-28 10:33   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 6/8] libxlu: free buffer in failure path for PCI related functions Wei Liu
2015-07-28 10:35   ` Ian Campbell
2015-07-27 17:45 ` [PATCH for-4.6 v2 7/8] python/xc: reinstate original implementation of next_bdf Wei Liu
2015-07-27 22:58   ` Andrew Cooper
2015-07-28  1:06   ` Chen, Tiejun [this message]
2015-07-28 10:38   ` Ian Campbell
2015-07-28 10:44     ` Wei Liu
2015-07-27 17:45 ` [PATCH for-4.6 v2 8/8] libxl: remove dead code libxl__domain_shutdown_reason Wei Liu
2015-07-28 10:52   ` Ian Campbell
2015-07-28 11:08 ` [PATCH for-4.6 v2 0/8] tools: fixes inspired by Coverity scan Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55B6D59E.6010600@intel.com \
    --to=tiejun.chen@intel.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.