Messages in this thread | | | Subject | Re: [PATCH] xen-blkfront: don't make discard-alignment mandatory | From | Jürgen Groß <> | Date | Tue, 19 Jan 2021 11:11:26 +0100 |
| |
On 19.01.21 11:06, Roger Pau Monné wrote: > On Tue, Jan 19, 2021 at 08:43:01AM +0100, Jürgen Groß wrote: >> On 18.01.21 16:15, Roger Pau Monne wrote: >>> Don't require the discard-alignment xenstore node to be present in >>> order to correctly setup the feature. This can happen with versions of >>> QEMU that only write the discard-granularity but not the >>> discard-alignment node. >>> >>> Assume discard-alignment is 0 if not present. While there also fix the >>> logic to not enable the discard feature if discard-granularity is not >>> present. >>> >>> Reported-by: Arthur Borsboom <arthurborsboom@gmail.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >>> Cc: Juergen Gross <jgross@suse.com> >>> Cc: Stefano Stabellini <sstabellini@kernel.org> >>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >>> Cc: "Roger Pau Monné" <roger.pau@citrix.com> >>> Cc: Jens Axboe <axboe@kernel.dk> >>> Cc: xen-devel@lists.xenproject.org >>> Cc: linux-block@vger.kernel.org >>> Cc: Arthur Borsboom <arthurborsboom@gmail.com> >>> --- >>> drivers/block/xen-blkfront.c | 25 +++++++++++++------------ >>> 1 file changed, 13 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c >>> index 5265975b3fba..5a93f7cc2939 100644 >>> --- a/drivers/block/xen-blkfront.c >>> +++ b/drivers/block/xen-blkfront.c >>> @@ -2179,22 +2179,23 @@ static void blkfront_closing(struct blkfront_info *info) >>> static void blkfront_setup_discard(struct blkfront_info *info) >>> { >>> - int err; >>> - unsigned int discard_granularity; >>> - unsigned int discard_alignment; >>> + unsigned int discard_granularity = 0; >>> + unsigned int discard_alignment = 0; >>> + unsigned int discard_secure = 0; >>> - info->feature_discard = 1; >>> - err = xenbus_gather(XBT_NIL, info->xbdev->otherend, >>> + xenbus_gather(XBT_NIL, info->xbdev->otherend, >>> "discard-granularity", "%u", &discard_granularity, >>> "discard-alignment", "%u", &discard_alignment, >>> + "discard-secure", "%u", &discard_secure, >>> NULL); >> >> This would mean that "discard-secure" will be evaluated only if the >> other two items are set in Xenstore. From blkif.h I can't see this is >> required, and your patch is modifying today's behavior in this regard. >> >> You might want to have three xenbus_read_unsigned() calls instead. > > You are right, discard-secure should be fetched regardless of whether > discard-alignment exists. > > I can fetch discard-granularity and discard-alignment using > xenbus_gather and keep discard-secure using xenbus_read_unsigned. Let > me send a new version.
I'm still not convinced this is correct. blkif.h doesn't mention that discard-alignment will be valid only with discard-granularity being present.
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |