LinuxLists.cc - [PATCH 00/12] xen: add common function for reading optional value

2016-10-31 16:48:46

Subject: [PATCH 00/12] xen: add common function for reading optional value

There are multiple instances of code reading an optional unsigned
parameter from Xenstore via xenbus_scanf(). Instead of repeating the
same code over and over add a service function doing the job and
replace the call of xenbus_scanf() with the call of the new function
where appropriate.

Juergen Gross (12):
xen: introduce xenbus_read_unsigned()
xen: make use of xenbus_read_unsigned() in xen-blkback
xen: make use of xenbus_read_unsigned() in xen-blkfront
xen: make use of xenbus_read_unsigned() in xen-tpmfront
xen: make use of xenbus_read_unsigned() in xen-kbdfront
xen: make use of xenbus_read_unsigned() in xen-netback
xen: make use of xenbus_read_unsigned() in xen-netfront
xen: make use of xenbus_read_unsigned() in xen-pcifront
xen: make use of xenbus_read_unsigned() in xen-scsifront
xen: make use of xenbus_read_unsigned() in xen-fbfront
xen: make use of xenbus_read_unsigned() in xen-pciback
xen: make use of xenbus_read_unsigned() in xenbus

drivers/block/xen-blkback/xenbus.c | 36 ++++++--------
drivers/block/xen-blkfront.c | 81 ++++++++++---------------------
drivers/char/tpm/xen-tpmfront.c | 8 +--
drivers/input/misc/xen-kbdfront.c | 13 ++---
drivers/net/xen-netback/xenbus.c | 50 ++++++-------------
drivers/net/xen-netfront.c | 67 +++++++------------------
drivers/pci/xen-pcifront.c | 6 +--
drivers/scsi/xen-scsifront.c | 6 +--
drivers/video/fbdev/xen-fbfront.c | 13 ++---
drivers/xen/xen-pciback/xenbus.c | 8 ++-
drivers/xen/xenbus/xenbus_probe_backend.c | 8 +--
drivers/xen/xenbus/xenbus_xs.c | 22 +++++++--
include/xen/xenbus.h | 4 ++
13 files changed, 112 insertions(+), 210 deletions(-)

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
2.6.6

2016-10-31 16:48:51

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 02/12] xen: make use of xenbus_read_unsigned() in xen-blkback

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of one read from int to unsigned,
but this case has been wrong before: negative values are not allowed
for the modified case.

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/block/xen-blkback/xenbus.c | 36 ++++++++++++++----------------------
1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3cc6d1d..415e79b 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -533,13 +533,11 @@ static void xen_blkbk_discard(struct xenbus_transaction xbt, struct backend_info
struct xenbus_device *dev = be->dev;
struct xen_blkif *blkif = be->blkif;
int err;
- int state = 0, discard_enable;
+ int state = 0;
struct block_device *bdev = be->blkif->vbd.bdev;
struct request_queue *q = bdev_get_queue(bdev);

- err = xenbus_scanf(XBT_NIL, dev->nodename, "discard-enable", "%d",
- &discard_enable);
- if (err == 1 && !discard_enable)
+ if (!xenbus_read_unsigned(dev->nodename, "discard-enable", 1))
return;

if (blk_queue_discard(q)) {
@@ -1039,30 +1037,24 @@ static int connect_ring(struct backend_info *be)
xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
return -ENOSYS;
}
- err = xenbus_scanf(XBT_NIL, dev->otherend,
- "feature-persistent", "%u", &pers_grants);
- if (err <= 0)
- pers_grants = 0;
-
+ pers_grants = xenbus_read_unsigned(dev->otherend, "feature-persistent",
+ 0);
be->blkif->vbd.feature_gnt_persistent = pers_grants;
be->blkif->vbd.overflow_max_grants = 0;

/*
* Read the number of hardware queues from frontend.
*/
- err = xenbus_scanf(XBT_NIL, dev->otherend, "multi-queue-num-queues",
- "%u", &requested_num_queues);
- if (err < 0) {
- requested_num_queues = 1;
- } else {
- if (requested_num_queues > xenblk_max_queues
- || requested_num_queues == 0) {
- /* Buggy or malicious guest. */
- xenbus_dev_fatal(dev, err,
- "guest requested %u queues, exceeding the maximum of %u.",
- requested_num_queues, xenblk_max_queues);
- return -ENOSYS;
- }
+ requested_num_queues = xenbus_read_unsigned(dev->otherend,
+ "multi-queue-num-queues",
+ 1);
+ if (requested_num_queues > xenblk_max_queues
+ || requested_num_queues == 0) {
+ /* Buggy or malicious guest. */
+ xenbus_dev_fatal(dev, err,
+ "guest requested %u queues, exceeding the maximum of %u.",
+ requested_num_queues, xenblk_max_queues);
+ return -ENOSYS;
}
be->blkif->nr_rings = requested_num_queues;
if (xen_blkif_alloc_rings(be->blkif))
--
2.6.6

2016-10-31 16:49:03

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 05/12] xen: make use of xenbus_read_unsigned() in xen-kbdfront

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of the reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/input/misc/xen-kbdfront.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 227fbd2..3900875 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -108,7 +108,8 @@ static irqreturn_t input_handler(int rq, void *dev_id)
static int xenkbd_probe(struct xenbus_device *dev,
const struct xenbus_device_id *id)
{
- int ret, i, abs;
+ int ret, i;
+ unsigned int abs;
struct xenkbd_info *info;
struct input_dev *kbd, *ptr;

@@ -127,8 +128,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
if (!info->page)
goto error_nomem;

- if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-abs-pointer", "%d", &abs) < 0)
- abs = 0;
+ abs = xenbus_read_unsigned(dev->otherend, "feature-abs-pointer", 0);
if (abs) {
ret = xenbus_write(XBT_NIL, dev->nodename,
"request-abs-pointer", "1");
@@ -322,11 +322,8 @@ static void xenkbd_backend_changed(struct xenbus_device *dev,

case XenbusStateInitWait:
InitWait:
- ret = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "feature-abs-pointer", "%d", &val);
- if (ret < 0)
- val = 0;
- if (val) {
+ if (xenbus_read_unsigned(info->xbdev->otherend,
+ "feature-abs-pointer", 0)) {
ret = xenbus_write(XBT_NIL, info->xbdev->nodename,
"request-abs-pointer", "1");
if (ret)
--
2.6.6

2016-10-31 16:49:00

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 07/12] xen: make use of xenbus_read_unsigned() in xen-netfront

2016-10-31 16:48:57

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 08/12] xen: make use of xenbus_read_unsigned() in xen-pcifront

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of the read from int to unsigned,
but this case has been wrong before: negative values are not allowed
for the modified case.

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/pci/xen-pcifront.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index d6ff5e8..8fc2e95 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -1038,10 +1038,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
err = -ENOMEM;
goto out;
}
- err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, str, "%d",
- &state);
- if (err != 1)
- state = XenbusStateUnknown;
+ state = xenbus_read_unsigned(pdev->xdev->otherend, str,
+ XenbusStateUnknown);

if (state != XenbusStateClosing)
continue;
--
2.6.6

2016-10-31 16:49:30

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback

2016-10-31 16:49:50

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 12/12] xen: make use of xenbus_read_unsigned() in xenbus

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of the reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/xenbus/xenbus_probe_backend.c | 8 +-------
drivers/xen/xenbus/xenbus_xs.c | 7 +++----
2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c
index 04f7f85..37929df 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -224,13 +224,7 @@ static int read_frontend_details(struct xenbus_device *xendev)

int xenbus_dev_is_online(struct xenbus_device *dev)
{
- int rc, val;
-
- rc = xenbus_scanf(XBT_NIL, dev->nodename, "online", "%d", &val);
- if (rc != 1)
- val = 0; /* no online node present */
-
- return val;
+ return !!xenbus_read_unsigned(dev->nodename, "online", 0);
}
EXPORT_SYMBOL_GPL(xenbus_dev_is_online);

diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 99dfdfa..6afb993 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -687,7 +687,7 @@ static bool xen_strict_xenbus_quirk(void)
}
static void xs_reset_watches(void)
{
- int err, supported = 0;
+ int err;

if (!xen_hvm_domain() || xen_initial_domain())
return;
@@ -695,9 +695,8 @@ static void xs_reset_watches(void)
if (xen_strict_xenbus_quirk())
return;

- err = xenbus_scanf(XBT_NIL, "control",
- "platform-feature-xs_reset_watches", "%d", &supported);
- if (err != 1 || !supported)
+ if (!xenbus_read_unsigned("control",
+ "platform-feature-xs_reset_watches", 0))
return;

err = xs_error(xs_single(XBT_NIL, XS_RESET_WATCHES, "", NULL));
--
2.6.6

2016-10-31 16:50:09

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 11/12] xen: make use of xenbus_read_unsigned() in xen-pciback

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of the read from int to unsigned,
but this case has been wrong before: negative values are not allowed
for the modified case.

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/xen-pciback/xenbus.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index 5ce878c..3f0aee0 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -362,7 +362,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
int err = 0;
int num_devs;
int domain, bus, slot, func;
- int substate;
+ unsigned int substate;
int i, len;
char state_str[64];
char dev_str[64];
@@ -395,10 +395,8 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
"configuration");
goto out;
}
- err = xenbus_scanf(XBT_NIL, pdev->xdev->nodename, state_str,
- "%d", &substate);
- if (err != 1)
- substate = XenbusStateUnknown;
+ substate = xenbus_read_unsigned(pdev->xdev->nodename, state_str,
+ XenbusStateUnknown);

switch (substate) {
case XenbusStateInitialising:
--
2.6.6

2016-10-31 16:50:11

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 10/12] xen: make use of xenbus_read_unsigned() in xen-fbfront

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of the reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/video/fbdev/xen-fbfront.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 0567d51..d0115a7 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -633,7 +633,6 @@ static void xenfb_backend_changed(struct xenbus_device *dev,
enum xenbus_state backend_state)
{
struct xenfb_info *info = dev_get_drvdata(&dev->dev);
- int val;

switch (backend_state) {
case XenbusStateInitialising:
@@ -657,16 +656,12 @@ static void xenfb_backend_changed(struct xenbus_device *dev,
if (dev->state != XenbusStateConnected)
goto InitWait; /* no InitWait seen yet, fudge it */

- if (xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "request-update", "%d", &val) < 0)
- val = 0;
- if (val)
+ if (xenbus_read_unsigned(info->xbdev->otherend,
+ "request-update", 0))
info->update_wanted = 1;

- if (xenbus_scanf(XBT_NIL, dev->otherend,
- "feature-resize", "%d", &val) < 0)
- val = 0;
- info->feature_resize = val;
+ info->feature_resize = xenbus_read_unsigned(dev->otherend,
+ "feature-resize", 0);
break;

case XenbusStateClosed:
--
2.6.6

2016-10-31 16:50:17

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 04/12] xen: make use of xenbus_read_unsigned() in xen-tpmfront

2016-10-31 16:50:15

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 09/12] xen: make use of xenbus_read_unsigned() in xen-scsifront

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/scsi/xen-scsifront.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 9dc8687..7e817c6 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -1060,13 +1060,9 @@ static void scsifront_read_backend_params(struct xenbus_device *dev,
struct vscsifrnt_info *info)
{
unsigned int sg_grant, nr_segs;
- int ret;
struct Scsi_Host *host = info->host;

- ret = xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg-grant", "%u",
- &sg_grant);
- if (ret != 1)
- sg_grant = 0;
+ sg_grant = xenbus_read_unsigned(dev->otherend, "feature-sg-grant", 0);
nr_segs = min_t(unsigned int, sg_grant, SG_ALL);
nr_segs = max_t(unsigned int, nr_segs, VSCSIIF_SG_TABLESIZE);
nr_segs = min_t(unsigned int, nr_segs,
--
2.6.6

2016-10-31 16:51:04

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 03/12] xen: make use of xenbus_read_unsigned() in xen-blkfront

Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
This requires to change the type of some reads from int to unsigned,
but these cases have been wrong before: negative values are not allowed
for the modified cases.

Cc: [email protected]
Cc: [email protected]

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/block/xen-blkfront.c | 81 ++++++++++++++------------------------------
1 file changed, 26 insertions(+), 55 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 9908597..2ee9646 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1758,17 +1758,13 @@ static int talk_to_blkback(struct xenbus_device *dev,
const char *message = NULL;
struct xenbus_transaction xbt;
int err;
- unsigned int i, max_page_order = 0;
- unsigned int ring_page_order = 0;
+ unsigned int i, max_page_order;
+ unsigned int ring_page_order;

- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "max-ring-page-order", "%u", &max_page_order);
- if (err != 1)
- info->nr_ring_pages = 1;
- else {
- ring_page_order = min(xen_blkif_max_ring_order, max_page_order);
- info->nr_ring_pages = 1 << ring_page_order;
- }
+ max_page_order = xenbus_read_unsigned(info->xbdev->otherend,
+ "max-ring-page-order", 0);
+ ring_page_order = min(xen_blkif_max_ring_order, max_page_order);
+ info->nr_ring_pages = 1 << ring_page_order;

for (i = 0; i < info->nr_rings; i++) {
struct blkfront_ring_info *rinfo = &info->rinfo[i];
@@ -1877,18 +1873,14 @@ static int talk_to_blkback(struct xenbus_device *dev,

static int negotiate_mq(struct blkfront_info *info)
{
- unsigned int backend_max_queues = 0;
- int err;
+ unsigned int backend_max_queues;
unsigned int i;

BUG_ON(info->nr_rings);

/* Check if backend supports multiple queues. */
- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "multi-queue-max-queues", "%u", &backend_max_queues);
- if (err < 0)
- backend_max_queues = 1;
-
+ backend_max_queues = xenbus_read_unsigned(info->xbdev->otherend,
+ "multi-queue-max-queues", 1);
info->nr_rings = min(backend_max_queues, xen_blkif_max_queues);
/* We need at least one ring. */
if (!info->nr_rings)
@@ -2195,7 +2187,6 @@ static void blkfront_setup_discard(struct blkfront_info *info)
int err;
unsigned int discard_granularity;
unsigned int discard_alignment;
- unsigned int discard_secure;

info->feature_discard = 1;
err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
@@ -2206,10 +2197,9 @@ static void blkfront_setup_discard(struct blkfront_info *info)
info->discard_granularity = discard_granularity;
info->discard_alignment = discard_alignment;
}
- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "discard-secure", "%u", &discard_secure);
- if (err > 0)
- info->feature_secdiscard = !!discard_secure;
+ info->feature_secdiscard =
+ !!xenbus_read_unsigned(info->xbdev->otherend, "discard-secure",
+ 0);
}

static int blkfront_setup_indirect(struct blkfront_ring_info *rinfo)
@@ -2301,16 +2291,11 @@ static int blkfront_setup_indirect(struct blkfront_ring_info *rinfo)
*/
static void blkfront_gather_backend_features(struct blkfront_info *info)
{
- int err;
- int barrier, flush, discard, persistent;
unsigned int indirect_segments;

info->feature_flush = 0;
info->feature_fua = 0;

- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "feature-barrier", "%d", &barrier);
-
/*
* If there's no "feature-barrier" defined, then it means
* we're dealing with a very old backend which writes
@@ -2318,7 +2303,7 @@ static void blkfront_gather_backend_features(struct blkfront_info *info)
*
* If there are barriers, then we use flush.
*/
- if (err > 0 && barrier) {
+ if (xenbus_read_unsigned(info->xbdev->otherend, "feature-barrier", 0)) {
info->feature_flush = 1;
info->feature_fua = 1;
}
@@ -2327,35 +2312,23 @@ static void blkfront_gather_backend_features(struct blkfront_info *info)
* And if there is "feature-flush-cache" use that above
* barriers.
*/
- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "feature-flush-cache", "%d", &flush);
-
- if (err > 0 && flush) {
+ if (xenbus_read_unsigned(info->xbdev->otherend, "feature-flush-cache",
+ 0)) {
info->feature_flush = 1;
info->feature_fua = 0;
}

- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "feature-discard", "%d", &discard);
-
- if (err > 0 && discard)
+ if (xenbus_read_unsigned(info->xbdev->otherend, "feature-discard", 0))
blkfront_setup_discard(info);

- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "feature-persistent", "%d", &persistent);
- if (err <= 0)
- info->feature_persistent = 0;
- else
- info->feature_persistent = persistent;
+ info->feature_persistent =
+ xenbus_read_unsigned(info->xbdev->otherend,
+ "feature-persistent", 0);

- 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,
- xen_blkif_max_segments);
+ indirect_segments = xenbus_read_unsigned(info->xbdev->otherend,
+ "feature-max-indirect-segments", 0);
+ info->max_indirect_segments = min(indirect_segments,
+ xen_blkif_max_segments);
}

/*
@@ -2420,11 +2393,9 @@ static void blkfront_connect(struct blkfront_info *info)
* provide this. Assume physical sector size to be the same as
* sector_size in that case.
*/
- err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
- "physical-sector-size", "%u", &physical_sector_size);
- if (err != 1)
- physical_sector_size = sector_size;
-
+ physical_sector_size = xenbus_read_unsigned(info->xbdev->otherend,
+ "physical-sector-size",
+ sector_size);
blkfront_gather_backend_features(info);
for (i = 0; i < info->nr_rings; i++) {
err = blkfront_setup_indirect(&info->rinfo[i]);
--
2.6.6

2016-10-31 16:51:29

by Juergen Gross

[permalink] [raw]

Subject: [PATCH 01/12] xen: introduce xenbus_read_unsigned()

There are multiple instances of code reading an optional unsigned
parameter from Xenstore via xenbus_scanf(). Instead of repeating the
same code over and over add a service function doing the job.

Signed-off-by: Juergen Gross <[email protected]>
---
drivers/xen/xenbus/xenbus_xs.c | 15 +++++++++++++++
include/xen/xenbus.h | 4 ++++
2 files changed, 19 insertions(+)

diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 22f7cd7..99dfdfa 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -559,6 +559,21 @@ int xenbus_scanf(struct xenbus_transaction t,
}
EXPORT_SYMBOL_GPL(xenbus_scanf);

+/* Read an (optional) unsigned value. */
+unsigned int xenbus_read_unsigned(const char *dir, const char *node,
+ unsigned int default_val)
+{
+ unsigned int val;
+ int ret;
+
+ ret = xenbus_scanf(XBT_NIL, dir, node, "%u", &val);
+ if (ret <= 0)
+ val = default_val;
+
+ return val;
+}
+EXPORT_SYMBOL_GPL(xenbus_read_unsigned);
+
/* Single printf and write: returns -errno or 0. */
int xenbus_printf(struct xenbus_transaction t,
const char *dir, const char *node, const char *fmt, ...)
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 32b944b..271ba62 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -151,6 +151,10 @@ __scanf(4, 5)
int xenbus_scanf(struct xenbus_transaction t,
const char *dir, const char *node, const char *fmt, ...);

+/* Read an (optional) unsigned value. */
+unsigned int xenbus_read_unsigned(const char *dir, const char *node,
+ unsigned int default_val);
+
/* Single printf and write: returns -errno or 0. */
__printf(4, 5)
int xenbus_printf(struct xenbus_transaction t,
--
2.6.6

2016-10-31 17:09:30

by David Miller

[permalink] [raw]

Subject: Re: [PATCH 00/12] xen: add common function for reading optional value

From: Juergen Gross <[email protected]>
Date: Mon, 31 Oct 2016 17:48:18 +0100

> There are multiple instances of code reading an optional unsigned
> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> same code over and over add a service function doing the job and
> replace the call of xenbus_scanf() with the call of the new function
> where appropriate.

As this seems to be a series that will go through some tree other
than mine, I assume the networking bits will be taken care of that
way.

2016-10-31 18:18:57

by Bjorn Helgaas

[permalink] [raw]

Subject: Re: [PATCH 08/12] xen: make use of xenbus_read_unsigned() in xen-pcifront

On Mon, Oct 31, 2016 at 05:48:26PM +0100, Juergen Gross wrote:
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of the read from int to unsigned,
> but this case has been wrong before: negative values are not allowed
> for the modified case.
>
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Juergen Gross <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

I assume this will be merged with the rest of the series via some tree
other than mine.

> ---
> drivers/pci/xen-pcifront.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index d6ff5e8..8fc2e95 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -1038,10 +1038,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
> err = -ENOMEM;
> goto out;
> }
> - err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, str, "%d",
> - &state);
> - if (err != 1)
> - state = XenbusStateUnknown;
> + state = xenbus_read_unsigned(pdev->xdev->otherend, str,
> + XenbusStateUnknown);
>
> if (state != XenbusStateClosing)
> continue;
> --
> 2.6.6
>

2016-11-01 04:33:12

by Juergen Gross

[permalink] [raw]

Subject: Re: [PATCH 00/12] xen: add common function for reading optional value

On 31/10/16 18:08, David Miller wrote:
> From: Juergen Gross <[email protected]>
> Date: Mon, 31 Oct 2016 17:48:18 +0100
>
>> There are multiple instances of code reading an optional unsigned
>> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
>> same code over and over add a service function doing the job and
>> replace the call of xenbus_scanf() with the call of the new function
>> where appropriate.
>
> As this seems to be a series that will go through some tree other
> than mine, I assume the networking bits will be taken care of that
> way.
>

If accepted I expect this series to go through the Xen tree.

Juergen

2016-11-01 09:42:30

by Paul Durrant

[permalink] [raw]

Subject: RE: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-netback

> -----Original Message-----
> From: Juergen Gross [mailto:[email protected]]
> Sent: 31 October 2016 16:48
> To: [email protected]; [email protected]
> Cc: David Vrabel <[email protected]>; [email protected];
> Juergen Gross <[email protected]>; Wei Liu <[email protected]>; Paul
> Durrant <[email protected]>; [email protected]
> Subject: [PATCH 06/12] xen: make use of xenbus_read_unsigned() in xen-
> netback
>
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of some reads from int to unsigned,
> but these cases have been wrong before: negative values are not allowed
> for the modified cases.
>
> Cc: [email protected]
> Cc: [email protected]

Reviewed-by: Paul Durrant <[email protected]>

> Cc: [email protected]
>
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> drivers/net/xen-netback/xenbus.c | 50 +++++++++++---------------------------
> --
> 1 file changed, 14 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> index 8674e18..7356e00 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -785,12 +785,9 @@ static void xen_mcast_ctrl_changed(struct
> xenbus_watch *watch,
> struct xenvif *vif = container_of(watch, struct xenvif,
> mcast_ctrl_watch);
> struct xenbus_device *dev = xenvif_to_xenbus_device(vif);
> - int val;
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend,
> - "request-multicast-control", "%d", &val) < 0)
> - val = 0;
> - vif->multicast_control = !!val;
> + vif->multicast_control = !!xenbus_read_unsigned(dev->otherend,
> + "request-multicast-control", 0);
> }
>
> static int xen_register_mcast_ctrl_watch(struct xenbus_device *dev,
> @@ -934,12 +931,9 @@ static void connect(struct backend_info *be)
> /* Check whether the frontend requested multiple queues
> * and read the number requested.
> */
> - err = xenbus_scanf(XBT_NIL, dev->otherend,
> - "multi-queue-num-queues",
> - "%u", &requested_num_queues);
> - if (err < 0) {
> - requested_num_queues = 1; /* Fall back to single queue */
> - } else if (requested_num_queues > xenvif_max_queues) {
> + requested_num_queues = xenbus_read_unsigned(dev->otherend,
> + "multi-queue-num-queues", 1);
> + if (requested_num_queues > xenvif_max_queues) {
> /* buggy or malicious guest */
> xenbus_dev_fatal(dev, err,
> "guest requested %u queues, exceeding the
> maximum of %u.",
> @@ -1134,7 +1128,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
> struct xenvif *vif = be->vif;
> struct xenbus_device *dev = be->dev;
> unsigned int rx_copy;
> - int err, val;
> + int err;
>
> err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy",
> "%u",
> &rx_copy);
> @@ -1150,10 +1144,7 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
> if (!rx_copy)
> return -EOPNOTSUPP;
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend,
> - "feature-rx-notify", "%d", &val) < 0)
> - val = 0;
> - if (!val) {
> + if (!xenbus_read_unsigned(dev->otherend, "feature-rx-notify", 0)) {
> /* - Reduce drain timeout to poll more frequently for
> * Rx requests.
> * - Disable Rx stall detection.
> @@ -1162,34 +1153,21 @@ static int read_xenbus_vif_flags(struct
> backend_info *be)
> be->vif->stall_timeout = 0;
> }
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> - "%d", &val) < 0)
> - val = 0;
> - vif->can_sg = !!val;
> + vif->can_sg = !!xenbus_read_unsigned(dev->otherend, "feature-
> sg", 0);
>
> vif->gso_mask = 0;
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> - "%d", &val) < 0)
> - val = 0;
> - if (val)
> + if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv4", 0))
> vif->gso_mask |= GSO_BIT(TCPV4);
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv6",
> - "%d", &val) < 0)
> - val = 0;
> - if (val)
> + if (xenbus_read_unsigned(dev->otherend, "feature-gso-tcpv6", 0))
> vif->gso_mask |= GSO_BIT(TCPV6);
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> - "%d", &val) < 0)
> - val = 0;
> - vif->ip_csum = !val;
> + vif->ip_csum = !xenbus_read_unsigned(dev->otherend,
> + "feature-no-csum-offload", 0);
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-ipv6-csum-
> offload",
> - "%d", &val) < 0)
> - val = 0;
> - vif->ipv6_csum = !!val;
> + vif->ipv6_csum = !!xenbus_read_unsigned(dev->otherend,
> + "feature-ipv6-csum-offload",
> 0);
>
> return 0;
> }
> --
> 2.6.6

2016-11-02 10:34:20

by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [PATCH 04/12] xen: make use of xenbus_read_unsigned() in xen-tpmfront

On Mon, Oct 31, 2016 at 05:48:22PM +0100, Juergen Gross wrote:
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of one read from int to unsigned,
> but this case has been wrong before: negative values are not allowed
> for the modified case.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Juergen Gross <[email protected]>

Reviewed-by: Jarkko Sakkinen <[email protected]>

/Jarkko

> ---
> drivers/char/tpm/xen-tpmfront.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 62028f4..50072cc 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -337,18 +337,14 @@ static int tpmfront_resume(struct xenbus_device *dev)
> static void backend_changed(struct xenbus_device *dev,
> enum xenbus_state backend_state)
> {
> - int val;
> -
> switch (backend_state) {
> case XenbusStateInitialised:
> case XenbusStateConnected:
> if (dev->state == XenbusStateConnected)
> break;
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend,
> - "feature-protocol-v2", "%d", &val) < 0)
> - val = 0;
> - if (!val) {
> + if (!xenbus_read_unsigned(dev->otherend, "feature-protocol-v2",
> + 0)) {
> xenbus_dev_fatal(dev, -EINVAL,
> "vTPM protocol 2 required");
> return;
> --
> 2.6.6
>

2016-11-04 04:26:35

by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [PATCH 04/12] xen: make use of xenbus_read_unsigned() in xen-tpmfront

2016-11-07 11:02:47

by David Vrabel

[permalink] [raw]

Subject: Re: [Xen-devel] [PATCH 01/12] xen: introduce xenbus_read_unsigned()

2016-11-07 11:08:19

by David Vrabel

[permalink] [raw]

Subject: Re: [PATCH 00/12] xen: add common function for reading optional value

On 31/10/16 16:48, Juergen Gross wrote:
> There are multiple instances of code reading an optional unsigned
> parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> same code over and over add a service function doing the job and
> replace the call of xenbus_scanf() with the call of the new function
> where appropriate.

Acked-by: David Vrabel <[email protected]>

Please queue for the next release.

David

2016-11-07 16:20:58

by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [PATCH 00/12] xen: add common function for reading optional value

On Mon, Nov 07, 2016 at 11:08:09AM +0000, David Vrabel wrote:
> On 31/10/16 16:48, Juergen Gross wrote:
> > There are multiple instances of code reading an optional unsigned
> > parameter from Xenstore via xenbus_scanf(). Instead of repeating the
> > same code over and over add a service function doing the job and
> > replace the call of xenbus_scanf() with the call of the new function
> > where appropriate.
>
> Acked-by: David Vrabel <[email protected]>
>
> Please queue for the next release.

If you want this change to tpmdd, please resend it to tpmdd mailing
list and CC it to linux-security-module. Thanks.

> David

/Jarkko

2016-11-09 00:29:22

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH 05/12] xen: make use of xenbus_read_unsigned() in xen-kbdfront

On Mon, Oct 31, 2016 at 05:48:23PM +0100, Juergen Gross wrote:
> Use xenbus_read_unsigned() instead of xenbus_scanf() when possible.
> This requires to change the type of the reads from int to unsigned,
> but these cases have been wrong before: negative values are not allowed
> for the modified cases.
>
> Cc: [email protected]
> Cc: [email protected]
>
> Signed-off-by: Juergen Gross <[email protected]>

Acked-by: Dmitry Torokhov <[email protected]>

> ---
> drivers/input/misc/xen-kbdfront.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index 227fbd2..3900875 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -108,7 +108,8 @@ static irqreturn_t input_handler(int rq, void *dev_id)
> static int xenkbd_probe(struct xenbus_device *dev,
> const struct xenbus_device_id *id)
> {
> - int ret, i, abs;
> + int ret, i;
> + unsigned int abs;
> struct xenkbd_info *info;
> struct input_dev *kbd, *ptr;
>
> @@ -127,8 +128,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
> if (!info->page)
> goto error_nomem;
>
> - if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-abs-pointer", "%d", &abs) < 0)
> - abs = 0;
> + abs = xenbus_read_unsigned(dev->otherend, "feature-abs-pointer", 0);
> if (abs) {
> ret = xenbus_write(XBT_NIL, dev->nodename,
> "request-abs-pointer", "1");
> @@ -322,11 +322,8 @@ static void xenkbd_backend_changed(struct xenbus_device *dev,
>
> case XenbusStateInitWait:
> InitWait:
> - ret = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
> - "feature-abs-pointer", "%d", &val);
> - if (ret < 0)
> - val = 0;
> - if (val) {
> + if (xenbus_read_unsigned(info->xbdev->otherend,
> + "feature-abs-pointer", 0)) {
> ret = xenbus_write(XBT_NIL, info->xbdev->nodename,
> "request-abs-pointer", "1");
> if (ret)
> --
> 2.6.6
>

--
Dmitry