docs/man/xl-disk-configuration.5.pod.in | 25 +++++++++++++++++++++++++ tools/golang/xenlight/helpers.gen.go | 6 ++++++ tools/golang/xenlight/types.gen.go | 1 + tools/include/libxl.h | 7 +++++++ tools/libs/light/libxl_arm.c | 4 ++-- tools/libs/light/libxl_disk.c | 13 +++++++++++++ tools/libs/light/libxl_types.idl | 1 + tools/libs/util/libxlu_disk_l.l | 3 +++ 8 files changed, 58 insertions(+), 2 deletions(-)
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
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@epam.com>
---
In addition to "libxl: arm: Add grant_usage parameter for virtio devices"
https://github.com/xen-project/xen/commit/c14254065ff4826e34f714e1790eab5217368c38
---
docs/man/xl-disk-configuration.5.pod.in | 25 +++++++++++++++++++++++++
tools/golang/xenlight/helpers.gen.go | 6 ++++++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h | 7 +++++++
tools/libs/light/libxl_arm.c | 4 ++--
tools/libs/light/libxl_disk.c | 13 +++++++++++++
tools/libs/light/libxl_types.idl | 1 +
tools/libs/util/libxlu_disk_l.l | 3 +++
8 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in
index bc945cc517..3c035456d5 100644
--- a/docs/man/xl-disk-configuration.5.pod.in
+++ b/docs/man/xl-disk-configuration.5.pod.in
@@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual
device (vdev) is not passed to the guest in that case, but it still must be
specified for the internal purposes.
+=item B<grant_usage=BOOLEAN>
+
+=over 4
+
+=item Description
+
+Specifies the usage of Xen grants for accessing guest memory. Only applicable
+to specification "virtio".
+
+=item Supported values
+
+If this option is B<true>, the Xen grants are always enabled.
+If this option is B<false>, the Xen grants are always disabled.
+
+=item Mandatory
+
+No
+
+=item Default value
+
+If this option is missing, then the default grant setting will be used,
+i.e. enable grants if backend-domid != 0.
+
+=back
+
=back
=head1 COLO Parameters
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 35e209ff1b..768ab0f566 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1879,6 +1879,9 @@ x.ActiveDisk = C.GoString(xc.active_disk)
x.HiddenDisk = C.GoString(xc.hidden_disk)
if err := x.Trusted.fromC(&xc.trusted);err != nil {
return fmt.Errorf("converting field Trusted: %v", err)
+}
+if err := x.GrantUsage.fromC(&xc.grant_usage);err != nil {
+return fmt.Errorf("converting field GrantUsage: %v", err)
}
return nil}
@@ -1927,6 +1930,9 @@ if x.HiddenDisk != "" {
xc.hidden_disk = C.CString(x.HiddenDisk)}
if err := x.Trusted.toC(&xc.trusted); err != nil {
return fmt.Errorf("converting field Trusted: %v", err)
+}
+if err := x.GrantUsage.toC(&xc.grant_usage); err != nil {
+return fmt.Errorf("converting field GrantUsage: %v", err)
}
return nil
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 7907aa8999..0b712d2aa4 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -740,6 +740,7 @@ ColoExport string
ActiveDisk string
HiddenDisk string
Trusted Defbool
+GrantUsage Defbool
}
type DeviceNic struct {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f1652b1664..2b69e08466 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -578,6 +578,13 @@
*/
#define LIBXL_HAVE_DEVICE_DISK_SPECIFICATION 1
+/*
+ * LIBXL_HAVE_DISK_GRANT_USAGE indicates that the libxl_device_disk
+ * has 'grant_usage' field to specify the usage of Xen grants for
+ * the specification 'virtio'.
+ */
+#define LIBXL_HAVE_DISK_GRANT_USAGE 1
+
/*
* LIBXL_HAVE_CONSOLE_ADD_XENSTORE indicates presence of the function
* libxl_console_add_xenstore() in libxl.
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 1539191774..1cb89fa584 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -1372,12 +1372,12 @@ next_resize:
libxl_device_disk *disk = &d_config->disks[i];
if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
- if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID)
+ if (libxl_defbool_val(disk->grant_usage))
iommu_needed = true;
FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
disk->backend_domid,
- disk->backend_domid != LIBXL_TOOLSTACK_DOMID) );
+ libxl_defbool_val(disk->grant_usage)) );
}
}
diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index ea3623dd6f..f39f427091 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -181,6 +181,9 @@ static int libxl__device_disk_setdefault(libxl__gc *gc, uint32_t domid,
return ERROR_INVAL;
}
disk->transport = LIBXL_DISK_TRANSPORT_MMIO;
+
+ libxl_defbool_setdefault(&disk->grant_usage,
+ disk->backend_domid != LIBXL_TOOLSTACK_DOMID);
}
if (hotplug && disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
@@ -429,6 +432,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
flexarray_append(back, libxl__device_disk_string_of_transport(disk->transport));
flexarray_append_pair(back, "base", GCSPRINTF("%"PRIu64, disk->base));
flexarray_append_pair(back, "irq", GCSPRINTF("%u", disk->irq));
+ flexarray_append_pair(back, "grant_usage",
+ libxl_defbool_val(disk->grant_usage) ? "1" : "0");
}
flexarray_append(front, "backend-id");
@@ -623,6 +628,14 @@ 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(ERROR, "Missing xenstore node %s/grant_usage", libxl_path);
+ goto cleanup;
+ }
+ libxl_defbool_set(&disk->grant_usage, strtoul(tmp, NULL, 0));
}
disk->vdev = xs_read(ctx->xsh, XBT_NULL,
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 899ad30969..6d76f25528 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -803,6 +803,7 @@ libxl_device_disk = Struct("device_disk", [
("active_disk", string),
("hidden_disk", string),
("trusted", libxl_defbool),
+ ("grant_usage", libxl_defbool),
])
libxl_device_nic = Struct("device_nic", [
diff --git a/tools/libs/util/libxlu_disk_l.l b/tools/libs/util/libxlu_disk_l.l
index 6d53c093a3..f37dd443bd 100644
--- a/tools/libs/util/libxlu_disk_l.l
+++ b/tools/libs/util/libxlu_disk_l.l
@@ -220,6 +220,9 @@ hidden-disk=[^,]*,? { STRIP(','); SAVESTRING("hidden-disk", hidden_disk, FROMEQU
trusted,? { libxl_defbool_set(&DPC->disk->trusted, true); }
untrusted,? { libxl_defbool_set(&DPC->disk->trusted, false); }
+grant_usage=1,? { libxl_defbool_set(&DPC->disk->grant_usage, true); }
+grant_usage=0,? { libxl_defbool_set(&DPC->disk->grant_usage, false); }
+
/* the target magic parameter, eats the rest of the string */
target=.* { STRIP(','); SAVESTRING("target", pdev_path, FROMEQUALS); }
--
2.34.1On Fri, Feb 02, 2024 at 12:49:03PM +0200, Oleksandr Tyshchenko wrote:
> diff --git a/tools/libs/util/libxlu_disk_l.l b/tools/libs/util/libxlu_disk_l.l
> index 6d53c093a3..f37dd443bd 100644
> --- a/tools/libs/util/libxlu_disk_l.l
> +++ b/tools/libs/util/libxlu_disk_l.l
> @@ -220,6 +220,9 @@ hidden-disk=[^,]*,? { STRIP(','); SAVESTRING("hidden-disk", hidden_disk, FROMEQU
> trusted,? { libxl_defbool_set(&DPC->disk->trusted, true); }
> untrusted,? { libxl_defbool_set(&DPC->disk->trusted, false); }
>
> +grant_usage=1,? { libxl_defbool_set(&DPC->disk->grant_usage, true); }
> +grant_usage=0,? { libxl_defbool_set(&DPC->disk->grant_usage, false); }
For other boolean type for the disk, we have "trusted/untrusted",
"discard/no-discard", "direct-io-save/", but you are adding
"grant_usage=1/grant_usage=0". Is that fine? But I guess having the new
option spelled "grant_usage" might be better, so it match the other
virtio devices and the implementation. But maybe
"use-grant/no-use-grant" might be ok?
In any case, the implementation need to match the documentation, and
vice versa. See below.
> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in
> index bc945cc517..3c035456d5 100644
> --- a/docs/man/xl-disk-configuration.5.pod.in
> +++ b/docs/man/xl-disk-configuration.5.pod.in
> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual
> +=item B<grant_usage=BOOLEAN>
>
> +=over 4
> +
> +=item Description
> +
> +Specifies the usage of Xen grants for accessing guest memory. Only applicable
> +to specification "virtio".
> +
> +=item Supported values
> +
> +If this option is B<true>, the Xen grants are always enabled.
> +If this option is B<false>, the Xen grants are always disabled.
Unfortunately, this is wrong, the implementation in the patch only
support two values: 1 / 0, nothing else, and trying to write "true" or
"false" would lead to an error. (Well actually it's "grant_usage=1" or
"grant_usage=0", there's nothing that cut that string at the '='.)
Also, do we really need the extra verbal description of each value here?
Is simply having the following would be enough?
=item Supported values
1, 0
The description in "Description" section would hopefully be enough.
> +=item Mandatory
> +
> +No
> +
> +=item Default value
> +
> +If this option is missing, then the default grant setting will be used,
> +i.e. enable grants if backend-domid != 0.
> +
> +=back
> +
> diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
> index ea3623dd6f..f39f427091 100644
> --- a/tools/libs/light/libxl_disk.c
> +++ b/tools/libs/light/libxl_disk.c
> @@ -181,6 +181,9 @@ static int libxl__device_disk_setdefault(libxl__gc *gc, uint32_t domid,
> return ERROR_INVAL;
> }
> disk->transport = LIBXL_DISK_TRANSPORT_MMIO;
> +
> + libxl_defbool_setdefault(&disk->grant_usage,
> + disk->backend_domid != LIBXL_TOOLSTACK_DOMID);
> }
>
> if (hotplug && disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
> @@ -429,6 +432,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
> flexarray_append(back, libxl__device_disk_string_of_transport(disk->transport));
> flexarray_append_pair(back, "base", GCSPRINTF("%"PRIu64, disk->base));
> flexarray_append_pair(back, "irq", GCSPRINTF("%u", disk->irq));
> + flexarray_append_pair(back, "grant_usage",
> + libxl_defbool_val(disk->grant_usage) ? "1" : "0");
> }
>
> flexarray_append(front, "backend-id");
> @@ -623,6 +628,14 @@ 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(ERROR, "Missing xenstore node %s/grant_usage", libxl_path);
> + goto cleanup;
I wonder if it's such a good idea to make this node mandatory. Could we
just apply the default value if the path is missing? I don't think the
value is going to be used anyway because I don't think from_xenstore() is
used during guest creation, and it looks like "grant_usage" is only
useful during guest creation. Also, the "grant_usage" node isn't
mandatory in "libxl_virtio.c", so no need to do something different
for disk.
> + }
> + libxl_defbool_set(&disk->grant_usage, strtoul(tmp, NULL, 0));
> }
>
> disk->vdev = xs_read(ctx->xsh, XBT_NULL,
Thanks,
--
Anthony PERARD
On 05.02.24 17:10, Anthony PERARD wrote:
Hello Anthony
> On Fri, Feb 02, 2024 at 12:49:03PM +0200, Oleksandr Tyshchenko wrote:
>> diff --git a/tools/libs/util/libxlu_disk_l.l b/tools/libs/util/libxlu_disk_l.l
>> index 6d53c093a3..f37dd443bd 100644
>> --- a/tools/libs/util/libxlu_disk_l.l
>> +++ b/tools/libs/util/libxlu_disk_l.l
>> @@ -220,6 +220,9 @@ hidden-disk=[^,]*,? { STRIP(','); SAVESTRING("hidden-disk", hidden_disk, FROMEQU
>> trusted,? { libxl_defbool_set(&DPC->disk->trusted, true); }
>> untrusted,? { libxl_defbool_set(&DPC->disk->trusted, false); }
>>
>> +grant_usage=1,? { libxl_defbool_set(&DPC->disk->grant_usage, true); }
>> +grant_usage=0,? { libxl_defbool_set(&DPC->disk->grant_usage, false); }
>
> For other boolean type for the disk, we have "trusted/untrusted",
> "discard/no-discard", "direct-io-save/", but you are adding
> "grant_usage=1/grant_usage=0". Is that fine? But I guess having the new
> option spelled "grant_usage" might be better, so it match the other
> virtio devices and the implementation.
Yes, I noticed that how booleans are described for the disk. I decided
to use the same representation of this option as it was already used for
virtio=[...]. But I would be ok with other variants ...
But maybe
> "use-grant/no-use-grant" might be ok?
... like that, but preferably with leaving libxl_device_disk's field
named "grant_usage" (if no objection).
>
> In any case, the implementation need to match the documentation, and
> vice versa. See below.
Sure.
>
>> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in
>> index bc945cc517..3c035456d5 100644
>> --- a/docs/man/xl-disk-configuration.5.pod.in
>> +++ b/docs/man/xl-disk-configuration.5.pod.in
>> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual
>> +=item B<grant_usage=BOOLEAN>
>>
>> +=over 4
>> +
>> +=item Description
>> +
>> +Specifies the usage of Xen grants for accessing guest memory. Only applicable
>> +to specification "virtio".
>> +
>> +=item Supported values
>> +
>> +If this option is B<true>, the Xen grants are always enabled.
>> +If this option is B<false>, the Xen grants are always disabled.
>
> Unfortunately, this is wrong, the implementation in the patch only
> support two values: 1 / 0, nothing else, and trying to write "true" or
> "false" would lead to an error. (Well actually it's "grant_usage=1" or
> "grant_usage=0", there's nothing that cut that string at the '='.)
You are right, only 1 / 0 can be set unlike for virtio=[...] which seems
happy with false/true.
>
> Also, do we really need the extra verbal description of each value here?
> Is simply having the following would be enough?
>
> =item Supported values
>
> 1, 0
>
> The description in "Description" section would hopefully be enough.
I think, this makes sense.
So, shall I leave "grant_usage=1/grant_usage=0" or use proposed option
"use-grant/no-use-grant"?
>
>> +=item Mandatory
>> +
>> +No
>> +
>> +=item Default value
>> +
>> +If this option is missing, then the default grant setting will be used,
>> +i.e. enable grants if backend-domid != 0.
>> +
>> +=back
>> +
>> diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
>> index ea3623dd6f..f39f427091 100644
>> --- a/tools/libs/light/libxl_disk.c
>> +++ b/tools/libs/light/libxl_disk.c
>> @@ -181,6 +181,9 @@ static int libxl__device_disk_setdefault(libxl__gc *gc, uint32_t domid,
>> return ERROR_INVAL;
>> }
>> disk->transport = LIBXL_DISK_TRANSPORT_MMIO;
>> +
>> + libxl_defbool_setdefault(&disk->grant_usage,
>> + disk->backend_domid != LIBXL_TOOLSTACK_DOMID);
>> }
>>
>> if (hotplug && disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
>> @@ -429,6 +432,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
>> flexarray_append(back, libxl__device_disk_string_of_transport(disk->transport));
>> flexarray_append_pair(back, "base", GCSPRINTF("%"PRIu64, disk->base));
>> flexarray_append_pair(back, "irq", GCSPRINTF("%u", disk->irq));
>> + flexarray_append_pair(back, "grant_usage",
>> + libxl_defbool_val(disk->grant_usage) ? "1" : "0");
>> }
>>
>> flexarray_append(front, "backend-id");
>> @@ -623,6 +628,14 @@ 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(ERROR, "Missing xenstore node %s/grant_usage", libxl_path);
>> + goto cleanup;
>
> I wonder if it's such a good idea to make this node mandatory. Could we
> just apply the default value if the path is missing? I don't think the
> value is going to be used anyway because I don't think from_xenstore() is
> used during guest creation, and it looks like "grant_usage" is only
> useful during guest creation. Also, the "grant_usage" node isn't
> mandatory in "libxl_virtio.c", so no need to do something different
> for disk.
I agree with your analysis, no need to raise an error if missing, let's
apply a default value which is the result of "disk->backend_domid !=
LIBXL_TOOLSTACK_DOMID".
>
>> + }
>> + libxl_defbool_set(&disk->grant_usage, strtoul(tmp, NULL, 0));
>> }
>>
>> disk->vdev = xs_read(ctx->xsh, XBT_NULL,
>
> Thanks,
>
On Mon, Feb 05, 2024 at 04:52:16PM +0000, Oleksandr Tyshchenko wrote:
> On 05.02.24 17:10, Anthony PERARD wrote:
> > On Fri, Feb 02, 2024 at 12:49:03PM +0200, Oleksandr Tyshchenko wrote:
> >> +grant_usage=1,? { libxl_defbool_set(&DPC->disk->grant_usage, true); }
> >> +grant_usage=0,? { libxl_defbool_set(&DPC->disk->grant_usage, false); }
> >
> > For other boolean type for the disk, we have "trusted/untrusted",
> > "discard/no-discard", "direct-io-save/", but you are adding
> > "grant_usage=1/grant_usage=0". Is that fine? But I guess having the new
> > option spelled "grant_usage" might be better, so it match the other
> > virtio devices and the implementation.
>
>
> Yes, I noticed that how booleans are described for the disk. I decided
> to use the same representation of this option as it was already used for
> virtio=[...]. But I would be ok with other variants ...
>
>
> But maybe
> > "use-grant/no-use-grant" might be ok?
>
> ... like that, but preferably with leaving libxl_device_disk's field
> named "grant_usage" (if no objection).
>
> >> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in
> >> index bc945cc517..3c035456d5 100644
> >> --- a/docs/man/xl-disk-configuration.5.pod.in
> >> +++ b/docs/man/xl-disk-configuration.5.pod.in
> >> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual
> >> +=item B<grant_usage=BOOLEAN>
> >>
> >> +=over 4
> >> +
> >> +=item Description
> >> +
> >> +Specifies the usage of Xen grants for accessing guest memory. Only applicable
> >> +to specification "virtio".
> >> +
> >> +=item Supported values
> >> +
> >> +If this option is B<true>, the Xen grants are always enabled.
> >> +If this option is B<false>, the Xen grants are always disabled.
> >
> > Unfortunately, this is wrong, the implementation in the patch only
> > support two values: 1 / 0, nothing else, and trying to write "true" or
> > "false" would lead to an error. (Well actually it's "grant_usage=1" or
> > "grant_usage=0", there's nothing that cut that string at the '='.)
>
>
> You are right, only 1 / 0 can be set unlike for virtio=[...] which seems
> happy with false/true.
>
>
> >
> > Also, do we really need the extra verbal description of each value here?
> > Is simply having the following would be enough?
> >
> > =item Supported values
> >
> > 1, 0
> >
> > The description in "Description" section would hopefully be enough.
>
>
> I think, this makes sense.
>
> So, shall I leave "grant_usage=1/grant_usage=0" or use proposed option
> "use-grant/no-use-grant"?
Let's go with "grant_usage=*", at least this will be consistent with the
option for "virtio".
Cheers,
--
Anthony PERARD
On 06.02.24 12:27, Anthony PERARD wrote: Hello Anthony [snip] >>>> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in >>>> index bc945cc517..3c035456d5 100644 >>>> --- a/docs/man/xl-disk-configuration.5.pod.in >>>> +++ b/docs/man/xl-disk-configuration.5.pod.in >>>> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual >>>> +=item B<grant_usage=BOOLEAN> >>>> >>>> +=over 4 >>>> + >>>> +=item Description >>>> + >>>> +Specifies the usage of Xen grants for accessing guest memory. Only applicable >>>> +to specification "virtio". >>>> + >>>> +=item Supported values >>>> + >>>> +If this option is B<true>, the Xen grants are always enabled. >>>> +If this option is B<false>, the Xen grants are always disabled. >>> >>> Unfortunately, this is wrong, the implementation in the patch only >>> support two values: 1 / 0, nothing else, and trying to write "true" or >>> "false" would lead to an error. (Well actually it's "grant_usage=1" or >>> "grant_usage=0", there's nothing that cut that string at the '='.) >> >> >> You are right, only 1 / 0 can be set unlike for virtio=[...] which seems >> happy with false/true. >> >> >>> >>> Also, do we really need the extra verbal description of each value here? >>> Is simply having the following would be enough? >>> >>> =item Supported values >>> >>> 1, 0 >>> >>> The description in "Description" section would hopefully be enough. >> >> >> I think, this makes sense. >> >> So, shall I leave "grant_usage=1/grant_usage=0" or use proposed option >> "use-grant/no-use-grant"? > > Let's go with "grant_usage=*", at least this will be consistent with the > option for "virtio". thanks for the confirmation, will do > > Cheers, >
On 02-02-24, 12:49, Oleksandr Tyshchenko wrote: > diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in > index bc945cc517..3c035456d5 100644 > --- a/docs/man/xl-disk-configuration.5.pod.in > +++ b/docs/man/xl-disk-configuration.5.pod.in > @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual > device (vdev) is not passed to the guest in that case, but it still must be > specified for the internal purposes. > > +=item B<grant_usage=BOOLEAN> > + > +=over 4 > + > +=item Description > + > +Specifies the usage of Xen grants for accessing guest memory. Only applicable > +to specification "virtio". > + > +=item Supported values > + > +If this option is B<true>, the Xen grants are always enabled. > +If this option is B<false>, the Xen grants are always disabled. > + > +=item Mandatory > + > +No > + > +=item Default value > + > +If this option is missing, then the default grant setting will be used, > +i.e. enable grants if backend-domid != 0. > + > +=back > + > =back > > =head1 COLO Parameters I wonder if there is a way to avoid the duplication here and use the definition from: docs/man/xl.cfg.5.pod.in somehow ? -- viresh
On 02.02.24 13:03, Viresh Kumar wrote: Hello Viresh > On 02-02-24, 12:49, Oleksandr Tyshchenko wrote: >> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-configuration.5.pod.in >> index bc945cc517..3c035456d5 100644 >> --- a/docs/man/xl-disk-configuration.5.pod.in >> +++ b/docs/man/xl-disk-configuration.5.pod.in >> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please note, the virtual >> device (vdev) is not passed to the guest in that case, but it still must be >> specified for the internal purposes. >> >> +=item B<grant_usage=BOOLEAN> >> + >> +=over 4 >> + >> +=item Description >> + >> +Specifies the usage of Xen grants for accessing guest memory. Only applicable >> +to specification "virtio". >> + >> +=item Supported values >> + >> +If this option is B<true>, the Xen grants are always enabled. >> +If this option is B<false>, the Xen grants are always disabled. >> + >> +=item Mandatory >> + >> +No >> + >> +=item Default value >> + >> +If this option is missing, then the default grant setting will be used, >> +i.e. enable grants if backend-domid != 0. >> + >> +=back >> + >> =back >> >> =head1 COLO Parameters > > I wonder if there is a way to avoid the duplication here and use the definition > from: docs/man/xl.cfg.5.pod.in somehow ? That's good point. I am not 100% sure, but if we could use something like that it would be really nice. Let's see what other reviewers will say. =item B<grant_usage=BOOLEAN> =over 4 Specifies the usage of Xen grants for accessing guest memory. Only applicable to specification "virtio". Please see B<grant_usage> in L<xl.cfg(5)> for more information on this option. =back
© 2016 - 2024 Red Hat, Inc.