Re: qdisk vs. file as vbd type

["Roger Pau Monné" <roger.pau@citrix.com>]

On 10/01/14 15:47, 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.


Linux blkfront checks for "feature-discard" on xenstore before trying 
to enable discard:

1795         err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
1796                             "feature-discard", "%d", &discard,
1797                             NULL);
1798 
1799         if (!err && discard)
1800                 blkfront_setup_discard(info);

But then in blkfront_setup_discard:

1615         type = xenbus_read(XBT_NIL, info->xbdev->otherend, "type", NULL);
1616         if (IS_ERR(type))
1617                 return;
1618 
1619         info->feature_secdiscard = 0;
1620         if (strncmp(type, "phy", 3) == 0) {
1621                 err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
1622                         "discard-granularity", "%u", &discard_granularity,
1623                         "discard-alignment", "%u", &discard_alignment,
1624                         NULL);
1625                 if (!err) {
1626                         info->feature_discard = 1;
1627                         info->discard_granularity = discard_granularity;
1628                         info->discard_alignment = discard_alignment;
1629                 }
1630                 err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
1631                             "discard-secure", "%d", &discard_secure,
1632                             NULL);
1633                 if (!err)
1634                         info->feature_secdiscard = discard_secure;
1635 
1636         } else if (strncmp(type, "file", 4) == 0)
1637                 info->feature_discard = 1;

I don't understand why blkfront doesn't check directly for "discard-
granularity" and "discard-alignment" and setup discard based on those 
parameters. Also, even if the backend sets "feature-discard" but the 
type is not "phy" or "file" discard will not be set.

The commit message that introduced those changes contains no info 
regarding why discard is only enabled for "phy" or "file" backends. 
Ccing Konrad and the original author of the change.

Roger.