[PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

For single items being collected this should be preferred as being more
typesafe (as the compiler can check format string and to-be-written-to
variable match) and more efficient (requiring one less parameter to be
passed).

1: xenbus: prefer xenbus_scanf() over xenbus_gather()
2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
4: xen-netback: prefer xenbus_scanf() over xenbus_gather()

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Avoid commit messages to continue from subjects. Group into a series.

[PATCH v2 1/4] xenbus: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

For single items being collected this should be preferred as being more
typesafe (as the compiler can check format string and to-be-written-to
variable match) and more efficient (requiring one less parameter to be
passed).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Avoid commit message to continue from subject.
---
 drivers/xen/xenbus/xenbus_client.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/xen/xenbus/xenbus_client.c
+++ 4.7-rc6-prefer-xenbus_scanf/drivers/xen/xenbus/xenbus_client.c
@@ -926,9 +926,9 @@ EXPORT_SYMBOL_GPL(xenbus_unmap_ring);
  */
 enum xenbus_state xenbus_read_driver_state(const char *path)
 {
-	enum xenbus_state result;
-	int err = xenbus_gather(XBT_NIL, path, "state", "%d", &result, NULL);
-	if (err)
+	int result;
+
+	if (xenbus_scanf(XBT_NIL, path, "state", "%d", &result) != 1)
 		result = XenbusStateUnknown;
 
 	return result;

[PATCH v2 2/4] xen-blkback: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

For single items being collected this should be preferred as being more
typesafe (as the compiler can check format string and to-be-written-to
variable match) and more efficient (requiring one less parameter to be
passed).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
v2: Avoid commit message to continue from subject.
---
 drivers/block/xen-blkback/xenbus.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

--- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/block/xen-blkback/xenbus.c
+++ 4.7-rc6-prefer-xenbus_scanf/drivers/block/xen-blkback/xenbus.c
@@ -1022,9 +1022,9 @@ static int connect_ring(struct backend_i
 	pr_debug("%s %s\n", __func__, dev->otherend);
 
 	be->blkif->blk_protocol = BLKIF_PROTOCOL_DEFAULT;
-	err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
-			    "%63s", protocol, NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, dev->otherend, "protocol",
+			   "%63s", protocol);
+	if (err <= 0)
 		strcpy(protocol, "unspecified, assuming default");
 	else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
 		be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
@@ -1036,10 +1036,9 @@ static int connect_ring(struct backend_i
 		xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
 		return -ENOSYS;
 	}
-	err = xenbus_gather(XBT_NIL, dev->otherend,
-			    "feature-persistent", "%u",
-			    &pers_grants, NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, dev->otherend,
+			   "feature-persistent", "%u", &pers_grants);
+	if (err <= 0)
 		pers_grants = 0;
 
 	be->blkif->vbd.feature_gnt_persistent = pers_grants;

[PATCH v2 3/4] xen-blkfront: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

For single items being collected this should be preferred as being more
typesafe (as the compiler can check format string and to-be-written-to
variable match) and more efficient (requiring one less parameter to be
passed).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
v2: Avoid commit message to continue from subject.
---
 drivers/block/xen-blkfront.c |   43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

--- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/block/xen-blkfront.c
+++ 4.7-rc6-prefer-xenbus_scanf/drivers/block/xen-blkfront.c
@@ -2208,10 +2208,9 @@ static void blkfront_setup_discard(struc
 		info->discard_granularity = discard_granularity;
 		info->discard_alignment = discard_alignment;
 	}
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-		    "discard-secure", "%d", &discard_secure,
-		    NULL);
-	if (!err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "discard-secure", "%u", &discard_secure);
+	if (err > 0)
 		info->feature_secdiscard = !!discard_secure;
 }
 
@@ -2310,9 +2309,8 @@ static void blkfront_gather_backend_feat
 
 	info->feature_flush = 0;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-barrier", "%d", &barrier,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-barrier", "%d", &barrier);
 
 	/*
 	 * If there's no "feature-barrier" defined, then it means
@@ -2321,38 +2319,35 @@ static void blkfront_gather_backend_feat
 	 *
 	 * If there are barriers, then we use flush.
 	 */
-	if (!err && barrier)
+	if (err > 0 && barrier)
 		info->feature_flush = REQ_FLUSH | REQ_FUA;
 	/*
 	 * And if there is "feature-flush-cache" use that above
 	 * barriers.
 	 */
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-flush-cache", "%d", &flush,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-flush-cache", "%d", &flush);
 
-	if (!err && flush)
+	if (err > 0 && flush)
 		info->feature_flush = REQ_FLUSH;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-discard", "%d", &discard,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-discard", "%d", &discard);
 
-	if (!err && discard)
+	if (err > 0 && discard)
 		blkfront_setup_discard(info);
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-persistent", "%u", &persistent,
-			NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-persistent", "%d", &persistent);
+	if (err <= 0)
 		info->feature_persistent = 0;
 	else
 		info->feature_persistent = persistent;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			    "feature-max-indirect-segments", "%u", &indirect_segments,
-			    NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-max-indirect-segments", "%u",
+			   &indirect_segments);
+	if (err <= 0)
 		info->max_indirect_segments = 0;
 	else
 		info->max_indirect_segments = min(indirect_segments,

Re: [Xen-devel] [PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[Konrad Rzeszutek Wilk]

On Fri, Jul 08, 2016 at 06:21:52AM -0600, Jan Beulich wrote:
> For single items being collected this should be preferred as being more
> typesafe (as the compiler can check format string and to-be-written-to
> variable match) and more efficient (requiring one less parameter to be
> passed).
> 
> 1: xenbus: prefer xenbus_scanf() over xenbus_gather()
> 2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
> 3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
> 4: xen-netback: prefer xenbus_scanf() over xenbus_gather()
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Avoid commit messages to continue from subjects. Group into a series.

To confuse this, Roger and I are the block sub-maintainers, which
when we are happy, I send to Jens, while the rest go through Boris,David, and Juergen.

Anyhow, I've already committed and tested for regressions these:
79ef83a xen-blkback: constify instance of "struct attribute_group"
5e4d659 xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
e9d1ebe xen-blkback: prefer xenbus_scanf() over xenbus_gather()
5b3b1db xen-blkback: really don't leak mode property

And plan to send them to Jens. They will shortly be at my git tree
under 'stable/for-jens-4.8'.


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

Re: [Xen-devel] [PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

>>> On 08.07.16 at 16:17, <konrad.wilk@oracle.com> wrote:
> On Fri, Jul 08, 2016 at 06:21:52AM -0600, Jan Beulich wrote:
>> For single items being collected this should be preferred as being more
>> typesafe (as the compiler can check format string and to-be-written-to
>> variable match) and more efficient (requiring one less parameter to be
>> passed).
>> 
>> 1: xenbus: prefer xenbus_scanf() over xenbus_gather()
>> 2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
>> 3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
>> 4: xen-netback: prefer xenbus_scanf() over xenbus_gather()
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Avoid commit messages to continue from subjects. Group into a series.
> 
> To confuse this, Roger and I are the block sub-maintainers, which
> when we are happy, I send to Jens, while the rest go through Boris,David, 
> and Juergen.

Which is why originally I had sent all of these separately. Yet David
was pretty unhappy about that.

Jan

Re: [Xen-devel] [PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[Konrad Rzeszutek Wilk]

On Fri, Jul 08, 2016 at 09:06:36AM -0600, Jan Beulich wrote:
> >>> On 08.07.16 at 16:17, <konrad.wilk@oracle.com> wrote:
> > On Fri, Jul 08, 2016 at 06:21:52AM -0600, Jan Beulich wrote:
> >> For single items being collected this should be preferred as being more
> >> typesafe (as the compiler can check format string and to-be-written-to
> >> variable match) and more efficient (requiring one less parameter to be
> >> passed).
> >> 
> >> 1: xenbus: prefer xenbus_scanf() over xenbus_gather()
> >> 2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
> >> 3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
> >> 4: xen-netback: prefer xenbus_scanf() over xenbus_gather()
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> v2: Avoid commit messages to continue from subjects. Group into a series.
> > 
> > To confuse this, Roger and I are the block sub-maintainers, which
> > when we are happy, I send to Jens, while the rest go through Boris,David, 
> > and Juergen.
> 
> Which is why originally I had sent all of these separately. Yet David
> was pretty unhappy about that.

Aye!
I am not critizing, just saying that this is getting complicated and you
are in the unfortunate situation to have to deal with this - so want
to apoligize for you having to go through this gauntlet.

Re: [Xen-devel] [PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[David Vrabel]

On 08/07/16 15:17, Konrad Rzeszutek Wilk wrote:
> On Fri, Jul 08, 2016 at 06:21:52AM -0600, Jan Beulich wrote:
>> For single items being collected this should be preferred as being more
>> typesafe (as the compiler can check format string and to-be-written-to
>> variable match) and more efficient (requiring one less parameter to be
>> passed).
>>
>> 1: xenbus: prefer xenbus_scanf() over xenbus_gather()
>> 2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
>> 3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
>> 4: xen-netback: prefer xenbus_scanf() over xenbus_gather()
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Avoid commit messages to continue from subjects. Group into a series.
> 
> To confuse this, Roger and I are the block sub-maintainers, which
> when we are happy, I send to Jens, while the rest go through Boris,David, and Juergen.
> 
> Anyhow, I've already committed and tested for regressions these:
> 79ef83a xen-blkback: constify instance of "struct attribute_group"
> 5e4d659 xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
> e9d1ebe xen-blkback: prefer xenbus_scanf() over xenbus_gather()
> 5b3b1db xen-blkback: really don't leak mode property

If they're tree wide largely mechanical changes to Xen-related APIs I
prefer that they go via the Xen tree all together.

This saves the submitter chasing the individual subsystem maintainers.

David

Re: [Xen-devel] [PATCH v2 0/4] xen: prefer xenbus_scanf() over xenbus_gather()

[Konrad Rzeszutek Wilk]

On Fri, Jul 08, 2016 at 04:41:20PM +0100, David Vrabel wrote:
> On 08/07/16 15:17, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jul 08, 2016 at 06:21:52AM -0600, Jan Beulich wrote:
> >> For single items being collected this should be preferred as being more
> >> typesafe (as the compiler can check format string and to-be-written-to
> >> variable match) and more efficient (requiring one less parameter to be
> >> passed).
> >>
> >> 1: xenbus: prefer xenbus_scanf() over xenbus_gather()
> >> 2: xen-blkback: prefer xenbus_scanf() over xenbus_gather()
> >> 3: xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
> >> 4: xen-netback: prefer xenbus_scanf() over xenbus_gather()
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> v2: Avoid commit messages to continue from subjects. Group into a series.
> > 
> > To confuse this, Roger and I are the block sub-maintainers, which
> > when we are happy, I send to Jens, while the rest go through Boris,David, and Juergen.
> > 
> > Anyhow, I've already committed and tested for regressions these:
> > 79ef83a xen-blkback: constify instance of "struct attribute_group"
> > 5e4d659 xen-blkfront: prefer xenbus_scanf() over xenbus_gather()
> > e9d1ebe xen-blkback: prefer xenbus_scanf() over xenbus_gather()
> > 5b3b1db xen-blkback: really don't leak mode property
> 
> If they're tree wide largely mechanical changes to Xen-related APIs I
> prefer that they go via the Xen tree all together.
> 
> This saves the submitter chasing the individual subsystem maintainers.

Sure, but from a stricly SubmittingPatches point of view it would
fall on your to get the Acks from the other maintainers.

Either way, I am OK with those blkback and blkfront going through
your tree, albeit one will conflict with Jens 'for-4.8/drivers'.

> 
> David