Re: [Xen-devel] [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX

["Chentao (Boby)" <boby.chen@xxxxxxxxxx>]

Thanks roger and joe.

I will adopt your suggestions in my v2 patch.

On 2015/3/31 22:57, Roger Pau Monné wrote:
> El 31/03/15 a les 23.14, Tao Chen ha escrit:
> > Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
> > of DRV_PFX in the pr and DPRINTK sentences. It will simplify the code.
> >
> > And if the pr sentences miss a \n, add it in the end. If the DPRINTK
> > sentences have redundant \n, remove it. It will format the code.
> >
> > These all make the readability of the code become better.
> Thanks for the patch.
>
> > Signed-off-by: Tao Chen <boby.chen@xxxxxxxxxx>
> > ---
> >   drivers/block/xen-blkback/blkback.c | 62 ++++++++++++++++++-------------------
> >   drivers/block/xen-blkback/common.h  |  6 ----
> >   drivers/block/xen-blkback/xenbus.c  | 18 ++++++-----
> >   3 files changed, 42 insertions(+), 44 deletions(-)
> [...]
> > diff --git a/drivers/block/xen-blkback/common.h 
> > b/drivers/block/xen-blkback/common.h
> > index 375d288..f620b5d 100644
> > --- a/drivers/block/xen-blkback/common.h
> > +++ b/drivers/block/xen-blkback/common.h
> > @@ -44,12 +44,6 @@
> >   #include <xen/interface/io/blkif.h>
> >   #include <xen/interface/io/protocols.h>
> >
> > -#define DRV_PFX "xen-blkback:"
> > -#define DPRINTK(fmt, args...)                          \
> > -       pr_debug(DRV_PFX "(%s:%d) " fmt ".\n",              \
> > -                __func__, __LINE__, ##args)
> > -
> > -
> >   /*
> >    * This is the maximum number of segments that would be allowed in indirect
> >    * requests. This value will also be passed to the frontend.
> > diff --git a/drivers/block/xen-blkback/xenbus.c 
> > b/drivers/block/xen-blkback/xenbus.c
> > index b33083e..0dbbfeb 100644
> > --- a/drivers/block/xen-blkback/xenbus.c
> > +++ b/drivers/block/xen-blkback/xenbus.c
> > @@ -14,6 +14,11 @@
> >
> >   */
> >
> > +#define pr_fmt(fmt) "xen-blkback: " fmt
> > +#define DPRINTK(fmt, args...)                  \
> > +       pr_debug("(%s:%d) " fmt ".\n",              \
> > +               __func__, __LINE__, ##args)
> > +
> >   #include <stdarg.h>
> >   #include <linux/module.h>
> >   #include <linux/kthread.h>
> > @@ -426,14 +431,14 @@ static int xen_vbd_create(struct xen_blkif *blkif, 
> > blkif_vdev_t handle,
> >                                  FMODE_READ : FMODE_WRITE, NULL);
> >
> >         if (IS_ERR(bdev)) {
> > -               DPRINTK("xen_vbd_create: device %08x could not be opened.\n",
> > +               DPRINTK("xen_vbd_create: device %08x could not be opened",
> >                         vbd->pdevice);
> >                 return -ENOENT;
> >         }
> >
> >         vbd->bdev = bdev;
> >         if (vbd->bdev->bd_disk == NULL) {
> > -               DPRINTK("xen_vbd_create: device %08x doesn't exist.\n",
> > +               DPRINTK("xen_vbd_create: device %08x doesn't exist",
> >                         vbd->pdevice);
> IMHO this two above should be made a pr_warn probably...
>
> >                 xen_vbd_free(vbd);
> >                 return -ENOENT;
> > @@ -452,7 +457,7 @@ static int xen_vbd_create(struct xen_blkif *blkif, 
> > blkif_vdev_t handle,
> >         if (q && blk_queue_secdiscard(q))
> >                 vbd->discard_secure = true;
> >
> > -       DPRINTK("Successful creation of handle=%04x (dom=%u)\n",
> > +       DPRINTK("Successful creation of handle=%04x (dom=%u)",
> >                 handle, blkif->domid);
> ...and this should be turned into a plain pr_debug. And with that we can
> get rid of DPRINTK.
>
> Roger.
>
>
> .

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