Re: [Xen-devel] qdisk vs. file as vbd type

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-01-10 at 10:15 -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Jan 10, 2014 at 04:00:00PM +0100, Olaf Hering wrote:
> > On Fri, Jan 10, Ian Campbell wrote:
> > 
> > > On Fri, 2014-01-10 at 15:40 +0100, Olaf Hering wrote:
> > > > What is the reason the backend 'type' property of a configured disk is
> > > > now "qdisk" instead of "file"?
> > > 
> > > Because qdisk is the backend instead of loop+blk (==file) I think this
> > > just happens naturally.
> > > 
> > > >  Would the guest really care about that
> > > > detail?  For example block-front currently just checks for "phy" and
> > > > "file" when deciding if discard should be enabled.
> > > 
> > > That sounds entirely bogus, it should be checking for some sort of
> > > feature-discard.
> > 
> > It does that, then calls blkfront_setup_discard which in turn knows just
> > about phy and file. And I wonder why it does that.
> > Maybe this function should be simplified to assume that if its called
> > feature_discard can be enabled. And if both
> > discard-granularity/discard-alignment exist those properties should be
> > assigned, similar for discard-secure.
> > 
> > Now that I look at the history of blkfront_setup_discard:
> > 
> >  Li, Konrad, why does that function care at all about the 'type'?
> >  Shouldnt that check be removed?
> 
> You are looking at:
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1664)   } 
> else if (strncmp(type, "file", 4) == 0)
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1665)         
>   info->feature_discard = 1;
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1666)
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1667)   
> kfree(type);
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1668)}
> ed30bf317       (Li Dongyang    2011-09-01 18:39:09 +0800       1669)
> 
> My recollection is that at the time the patches were developed, loop
> was not able to do discard operations. That has since changed and
> loop can do it. Hence the force of =1 was added in.
> 
> But that assumes that 'file' is going through the 'loop' device.
> 
> If that assumption is incorrect then this needs to be fixed and
> perhaps the underlaying device ('file'?) interogated as to whether
> it can do discard or not.

Why on earth is this happening in the frontend?

The *backend* should be querying the underlying device and propagating
the result via the feature flag to the frontend. Having the backend
advertise discard and then have the frontend second guess this based on
rumour and hearsay (which is all probing this type field) is just nuts.

Ian.


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