On 04/28/22 13:37, Richard W.M. Jones wrote:
> It is likely this field was never used. Repurpose it as a general
> scratchpad for callers to use to save some per-drive data.
>
> Fixes: https://github.com/libguestfs/libguestfs/issues/80
> ---
> generator/actions_core.ml | 30 ++++++++++++++++++++++++++++--
> lib/drives.c | 20 ++++++++++++++++++++
> tests/c-api/tests-main.c | 4 +++-
> 3 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/generator/actions_core.ml b/generator/actions_core.ml
> index ce9ee39cca..44b0c22f29 100644
> --- a/generator/actions_core.ml
> +++ b/generator/actions_core.ml
> @@ -263,8 +263,9 @@ deprecated C<guestfs_add_drive_with_if> call (q.v.)
>
> =item C<name>
>
> -This field used to be passed as a hint for guest inspection, but
> -it is no longer used.
> +This is an arbitrary string. If you pass in a string here, you
> +can read it back by calling C<guestfs_get_drive_name>. It is
> +not used by libguestfs itself.
>
> =item C<label>
>
> @@ -1507,6 +1508,31 @@ This call is used by L<virt-rescue(1)> to write
> directly to
> appliance console (for passing through keystrokes). It should
> not normally be used by other libguestfs users." };
>
> + { defaults with
> + name = "get_drive_name"; added = (1, 49, 1);
> + style = RString (RPlainString, "name"), [Int "index"], [];
> + blocking = false;
> + tests = [
> + InitEmpty, Always, TestResult (
> + [["get_drive_name"; "0"]], "STREQ (ret, \"test\")"), [];
> + InitEmpty, Always, TestLastFail (
> + [["get_drive_name"; "1"]]), [];
> + InitEmpty, Always, TestLastFail (
> + [["get_drive_name"; "99"]]), [];
> + ];
> + shortdesc = "get the drive name";
> + longdesc = "\
> +Return the optional C<name> field passed to C<guestfs_add_drive_opts>
> +when n'th drive C<index> was added.
> +
> +If there is no C<name> field associated with the drive
> +then this function returns an error, with error code C<ESRCH>.
> +
> +If you only have a device name and not an index, you can call
> +C<guestfs_device_index>. If what you want instead is the
> +drive name as it appears to libguestfs, there is no function
> +for that, instead use C<\"/dev/sda\">, C<\"/dev/sdb\"> etc." };
> +
> ]
>
> let daemon_functions = [
> diff --git a/lib/drives.c b/lib/drives.c
> index fd95308d2d..0da730c734 100644
> --- a/lib/drives.c
> +++ b/lib/drives.c
> @@ -31,6 +31,7 @@
> #include <netdb.h>
> #include <arpa/inet.h>
> #include <assert.h>
> +#include <errno.h>
> #include <libintl.h>
>
> #include "c-ctype.h"
> @@ -594,6 +595,25 @@ guestfs_int_free_drives (guestfs_h *g)
> g->nr_drives = 0;
> }
>
> +char *
> +guestfs_impl_get_drive_name (guestfs_h *g, int i)
> +{
> + const char *name;
> +
> + if (i < 0 || i > g->nr_drives) {
The upper bound check is off by one; should be ">=".
> + guestfs_int_error_errno (g, EINVAL, _("drive index out of range"));
> + return NULL;
> + }
> +
> + name = g->drives[i]->name;
Based on the comments in "lib/guestfs-internal.h" and on the
ITER_DRIVES() macro:
> /* Array of drives added by add-drive* APIs.
> *
> * Before launch this list can be empty or contain some drives.
> *
> * During launch, a dummy slot may be added which represents the
> * slot taken up by the appliance drive.
> *
> * During shutdown, this list is deleted, so that each launch gets a
> * fresh set of drives (however callers: don't do this, create a new
> * handle each time).
> *
> * Always use ITER_DRIVES macro to iterate over this list!
> */
> struct drive **drives;
> size_t nr_drives;
>
> #define ITER_DRIVES(g,i,drv) \
> for (i = 0; i < (g)->nr_drives; ++i) \
> if (((drv) = (g)->drives[i]) != NULL)
I think we should check whether "g->drives[i]" is NULL, before we check
"g->drives[i]->name".
Otherwise, I'm OK with this patch (still need to look at the alternative
you psoted).
Thanks
Laszlo
> + if (!name) {
> + guestfs_int_error_errno (g, ESRCH, _("drive does not have a name"));
> + return NULL;
> + }
> +
> + return safe_strdup (g, name);
> +}
> +
> /**
> * Check string parameter matches regular expression
> * C<^[-_[:alnum:]]+$> (in C locale).
> diff --git a/tests/c-api/tests-main.c b/tests/c-api/tests-main.c
> index 9e01e4a3dc..a0aad1735d 100644
> --- a/tests/c-api/tests-main.c
> +++ b/tests/c-api/tests-main.c
> @@ -431,7 +431,9 @@ create_handle (void)
> exit (EXIT_FAILURE);
> }
>
> - if (guestfs_add_drive_scratch (g, INT64_C(2)*1024*1024*1024, -1) == -1) {
> + if (guestfs_add_drive_scratch (g, INT64_C(2)*1024*1024*1024,
> + GUESTFS_ADD_DRIVE_SCRATCH_NAME, "test",
> + -1) == -1) {
> printf ("FAIL: guestfs_add_drive_scratch\n");
> exit (EXIT_FAILURE);
> }
>
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs
