Re: qdisk vs. file as vbd type

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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.

> 
> Olaf