[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V2] libxl: Add "grant_usage" parameter for virtio disk devices


  • To: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • From: Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>
  • Date: Thu, 15 Feb 2024 13:08:34 +0000
  • Accept-language: en-US, ru-RU
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=esFLRZ0NxMYwaOuMwp9NhK/tOhIteumlkqPY4MKo7dg=; b=P4krCnyOYxSSGGxfO0QBYAdsIvyNv7vYtMjcoxorx08CzSJ2vsWH62yJdj3sSHzuBMz89Bre+ulY98ZHXo4KhWAgB606YRZCmLG7USReqPQTtVm4+oMUkBGHmliCM/cLz8kdzC0r8913DjgLUGh4a5ycQP7add3vrlaqdQuDe6E5DYY9UROH8MAt5lbRZ5H93R37H382uTndYr8IfKdrUiXlX7XlZjupN20EA+aMCngEzF6U4LIDEBPnoi47uZXcXVoIcYKwE8KsqPhy1IZAOGnWNxDDeewHzXtPd+6foU7y8jpnrhQ/x9Mts9oagKclkAN3VLKtkVXrG0xCCVcFoA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NKFD7ItsiQH7K0KwjtckM5PkY75Upx59C4EChvSknKpo4X6YsdhIy6ydpiUttT6GfeMSRMOQb5+jFTsuBfDqKCOUu8VL+VacnNpl1TBvR8hLqurQl/ByBm9rexAt+8GhkhU90oNscB25c/0UQh6rRozqFPmvQxkDZlXVebJW14cdpcvyXicfX3HVDk1rfRgfWYWZ+ifOWU6mjdrsswg7x6g8D3Gll810mqvD55SDmx2y7NCYB2QwNkLGyWzRG/JqIDbZlevFxVAxvBeX+A6e0HElzuAv6LUacojyubPFGqlpCdyHQFhKyJgthJ//ZgXU0eMjPwuCTRcr3Rdpr7qvrg==
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Nick Rosbrook <rosbrookn@xxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Viresh Kumar <viresh.kumar@xxxxxxxxxx>, Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>
  • Delivery-date: Thu, 15 Feb 2024 13:08:51 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHaWPlfVfBPDqFzI0qC9ZNM13q/+7EIOn0AgAMzrQA=
  • Thread-topic: [PATCH V2] libxl: Add "grant_usage" parameter for virtio disk devices


On 13.02.24 14:14, Anthony PERARD wrote:

Hello Anthony

> On Tue, Feb 06, 2024 at 02:38:14PM +0200, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>
>> Allow administrators to control whether Xen grant mappings for
>> the virtio disk devices should be used. By default (when new
>> parameter is not specified), the existing behavior is retained
>> (we enable grants if backend-domid != 0).
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>> ---
>> In addition to "libxl: arm: Add grant_usage parameter for virtio devices"
>> https://urldefense.com/v3/__https://github.com/xen-project/xen/commit/c14254065ff4826e34f714e1790eab5217368c38__;!!GF_29dbcQIUBPA!172qt30uI7DpsZcK-IpZu25JoQScPeF7do_MD-KEqJlo8gaH-TV1P_H2CYfsGI0j_l0GdUPPO4BjyUD2Q86Lk2IlF5zCiFWMAw$
>>  [github[.]com]
>>
>> I wonder, whether I had to also include autogenerated changes to:
>>   - tools/libs/util/libxlu_disk_l.c
>>   - tools/libs/util/libxlu_disk_l.h
> 
> Well, that could be done on commit. The changes are going to be needed
> to be committed as they may not be regenerated to include the new feature
> in a build.

Thanks. As V3 is needed anyway, I will include them.


> 
>> ---
>> diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
>> index ea3623dd6f..ed02b655a3 100644
>> --- a/tools/libs/light/libxl_disk.c
>> +++ b/tools/libs/light/libxl_disk.c
>> @@ -623,6 +628,15 @@ static int libxl__disk_from_xenstore(libxl__gc *gc, 
>> const char *libxl_path,
>>               goto cleanup;
>>           }
>>           disk->irq = strtoul(tmp, NULL, 10);
>> +
>> +        tmp = libxl__xs_read(gc, XBT_NULL,
>> +                             GCSPRINTF("%s/grant_usage", libxl_path));
>> +        if (!tmp) {
>> +            LOG(DEBUG, "Missing xenstore node %s/grant_usage, using default 
>> value", libxl_path);
> 
> Is this information useful for debugging?
> 
> It should be easy to find out if the grant_usage node is present or not
> by looking at xenstore, and I don't think libxl is going to make use of
> that information after this point, so I don't think that's going to be
> very useful.

It is not very useful, will drop the log.


> 
>> +            libxl_defbool_set(&disk->grant_usage,
>> +                              disk->backend_domid != LIBXL_TOOLSTACK_DOMID);
>> +        } else
>> +            libxl_defbool_set(&disk->grant_usage, strtoul(tmp, NULL, 0));
> 
> Per coding style, it's better to have both side of an if..else to have
> {}-block or none of them. So could you add a {} block in the else, or
> remove the {} from the true side if we remove the LOG()?


Will do.

> 
> Thanks,
> 

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.