Re: [Xen-devel] [PATCH v2] introduce a cache options for PV disks

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Stefano Stabellini writes ("Re: [PATCH v2] introduce a cache options for PV 
disks"):
> It's mostly OK, just few minor corrections for code readability

Thanks.

> >      int info = 0;
> > +    char *directiosafe == NULL;
>                            ^ =

I did warn that I hadn't compiled it :-).

> >  out_error:
> > @@ -773,6 +779,7 @@ out_error:
> >      blkdev->dev = NULL;
> >      g_free(blkdev->devtype);
> >      blkdev->devtype = NULL;
> > +    g_free(directiosafe);
> 
> maybe add
> 
> blkdev->directiosafe = false;

Sure, does no harm.

> > +    if (blkdev->directiosafe) {
> > +        qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> > +    }
> 
> Please change this into:
> 
> if (blkdev->directiosafe) {
>     qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> } else {
>     qflags = BDRV_O_CACHE_WB;
> }

I don't think that can be right.  The result would be that the value
non-directiosafe value of qflags is changed.  But I will break it
apart as you suggest.

Ian.

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