| Message ID | 20240104090055.27323-18-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools: enable xenstore-stubdom to use 9pfs | expand |
On Thu, Jan 4, 2024 at 4:02 AM Juergen Gross <jgross@suse.com> wrote: > > Add support for the new 9pfs backend "xen-9pfsd". For this backend type > the tag defaults to "Xen" and the host side path to > "/var/log/xen/guests/<dom-name>". > > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Jason Andryuk <jandryuk@gmail.com> > --- > V2: > - test max_files and max_open_files, too (Jason Andryuk) > --- > docs/man/xl.cfg.5.pod.in | 36 ++++++++++++++++++++++++++++++++++-- > tools/xl/xl_parse.c | 36 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 70 insertions(+), 2 deletions(-) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index 2e234b450e..80011adbf3 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -772,10 +772,16 @@ settings, from the following list: > > =over 4 > > +=item B<type=TYPE> > + > +The backendtype for the PV device. Supported values are B<qemu> and > +B<xen-9pfsd>. The default is B<qemu>. > + > =item B<tag=STRING> > > 9pfs tag to identify the filesystem share. The tag is needed on the > -guest side to mount it. > +guest side to mount it. For the backendtype of B<xen-9pfsd> the tag defaults to > +"Xen". Here and below it's "xen_9pfsd" as the type. I tried mounting a xen-9pfsd export in Linux, but mount failed: mount: /mnt/9p-b: mount(2) system call failed: Remove I/O error. dmesg(1) may have more information after failed mount system call. dmesg doesn't have any other details. /sys/bus/xen/devices/9pfs-2/state shows Connected. Regards, Jason
On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote: > @@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source, > > libxl_string_list_dispose(&pairs); > > + if (p9->type == LIBXL_P9_TYPE_UNKNOWN) { > + p9->type = LIBXL_P9_TYPE_QEMU; The defaulting is normally done in libxl, so that it works for all users of libxl. Can this be done instead in libxl? Hopefully, it's enough to do it in libxl__device_p9_setdefault(). Same question for the followup checks and default values. > + } > + if (p9->type == LIBXL_P9_TYPE_QEMU && > + (p9->max_files || p9->max_open_files || p9->max_space || > + p9->auto_delete)) { > + fprintf(stderr, "Illegal 9pfs parameter combination\n"); > + exit(1); > + } > + if (p9->type == LIBXL_P9_TYPE_XEN_9PFSD) { > + if (!p9->tag) { > + replace_string(&p9->tag, "Xen"); > + } > + if (!p9->path) { > + char *path; > + > + xasprintf(&path, XEN_LOG_DIR "/guests/%s", c_info->name); > + replace_string(&p9->path, path); > + free(path); > + } > + } Thanks,
On 15.01.24 16:14, Anthony PERARD wrote: > On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote: >> @@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source, >> >> libxl_string_list_dispose(&pairs); >> >> + if (p9->type == LIBXL_P9_TYPE_UNKNOWN) { >> + p9->type = LIBXL_P9_TYPE_QEMU; > > The defaulting is normally done in libxl, so that it works for all users > of libxl. Can this be done instead in libxl? Hopefully, it's enough to > do it in libxl__device_p9_setdefault(). > > Same question for the followup checks and default values. I'll look into it. Juergen
On 31.01.24 16:20, Jürgen Groß wrote: > On 15.01.24 16:14, Anthony PERARD wrote: >> On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote: >>> @@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source, >>> libxl_string_list_dispose(&pairs); >>> + if (p9->type == LIBXL_P9_TYPE_UNKNOWN) { >>> + p9->type = LIBXL_P9_TYPE_QEMU; >> >> The defaulting is normally done in libxl, so that it works for all users >> of libxl. Can this be done instead in libxl? Hopefully, it's enough to >> do it in libxl__device_p9_setdefault(). >> >> Same question for the followup checks and default values. > > I'll look into it. Turns out this would need some hack, as I have no access to the name of the domain being created in libxl__device_p9_setdefault(). And this is needed for setting the path default value. Do you have any idea how to resolve this issue, or are you fine to keep the patch as is? Juergen
On 02.02.24 16:28, Juergen Gross wrote: > On 31.01.24 16:20, Jürgen Groß wrote: >> On 15.01.24 16:14, Anthony PERARD wrote: >>> On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote: >>>> @@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source, >>>> libxl_string_list_dispose(&pairs); >>>> + if (p9->type == LIBXL_P9_TYPE_UNKNOWN) { >>>> + p9->type = LIBXL_P9_TYPE_QEMU; >>> >>> The defaulting is normally done in libxl, so that it works for all users >>> of libxl. Can this be done instead in libxl? Hopefully, it's enough to >>> do it in libxl__device_p9_setdefault(). >>> >>> Same question for the followup checks and default values. >> >> I'll look into it. > > Turns out this would need some hack, as I have no access to the name of > the domain being created in libxl__device_p9_setdefault(). And this is needed > for setting the path default value. > > Do you have any idea how to resolve this issue, or are you fine to keep the > patch as is? Just found a hopefully acceptable solution: if the type is XEN_9PFSD and no path has been specified I set it in xl_parse.c. The rest of the default settings and consistency checks are done in libxl. Is this okay for you? Juergen
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 2e234b450e..80011adbf3 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -772,10 +772,16 @@ settings, from the following list: =over 4 +=item B<type=TYPE> + +The backendtype for the PV device. Supported values are B<qemu> and +B<xen-9pfsd>. The default is B<qemu>. + =item B<tag=STRING> 9pfs tag to identify the filesystem share. The tag is needed on the -guest side to mount it. +guest side to mount it. For the backendtype of B<xen-9pfsd> the tag defaults to +"Xen". =item B<security_model="none"> @@ -785,12 +791,38 @@ squash or remap). =item B<path=STRING> -Filesystem path on the backend to export. +Filesystem path on the backend to export. For the backendtype of B<xen-9pfsd> +the path defaults to "@XEN_LOG_DIR@/guests/<guest-name>". =item B<backend=domain-id> Specify the backend domain name or id, defaults to dom0. +=item B<max-files=NUMBER> + +Specify the maximum number of files below B<path>. A value of 0 (which +is the default) doesn't limit the number of files. Only valid for +B<type=xen-9pfsd>. + +=item B<max-open-files=NUMBER> + +Specify the maximum number of concurrently opened files below B<path>. +Multiple opens of the same file are counted individually. Only valid for +B<type=xen-9pfsd>, which has a default of B<max-open-files=5>. + +=item B<max-space=NUMBER> + +Specify the maximum used disk space in MiB below B<path>. A value of 0 (which +is the default) doesn't limit the usable disk space. Only valid for +B<type=xen-9pfsd>. + +=item B<auto-delete=BOOLEAN> + +When set the backend will delete the oldest file which is currently not +opened by the guest in case the disk space limit set via B<max-space> or the +file limit set via B<max-files> is being reached. Only valid for +B<type=xen-9pfsd>. + =back =item B<pvcalls=[ "backend=domain-id", ... ]> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index ed983200c3..3ded557ebc 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -2232,6 +2232,20 @@ void parse_config_data(const char *config_source, replace_string(&p9->tag, value); } else if (!strcmp(key, "backend")) { replace_string(&p9->backend_domname, value); + } else if (!strcmp(key, "type")) { + if (libxl_p9_type_from_string(value, &p9->type)) { + fprintf(stderr, "failed to parse 9pfs type: %s\n", + value); + exit(1); + } + } else if (!strcmp(key, "max-files")) { + p9->max_files = parse_ulong(value); + } else if (!strcmp(key, "max-open-files")) { + p9->max_open_files = parse_ulong(value); + } else if (!strcmp(key, "max-space")) { + p9->max_space = parse_ulong(value); + } else if (!strcmp(key, "auto-delete")) { + p9->auto_delete = strtoul(value, NULL, 0); } else { fprintf(stderr, "Unknown 9pfs parameter '%s'\n", key); exit(1); @@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source, libxl_string_list_dispose(&pairs); + if (p9->type == LIBXL_P9_TYPE_UNKNOWN) { + p9->type = LIBXL_P9_TYPE_QEMU; + } + if (p9->type == LIBXL_P9_TYPE_QEMU && + (p9->max_files || p9->max_open_files || p9->max_space || + p9->auto_delete)) { + fprintf(stderr, "Illegal 9pfs parameter combination\n"); + exit(1); + } + if (p9->type == LIBXL_P9_TYPE_XEN_9PFSD) { + if (!p9->tag) { + replace_string(&p9->tag, "Xen"); + } + if (!p9->path) { + char *path; + + xasprintf(&path, XEN_LOG_DIR "/guests/%s", c_info->name); + replace_string(&p9->path, path); + free(path); + } + } + if (!p9->path || !p9->security_model || !p9->tag) { fprintf(stderr, "9pfs spec missing required field!\n"); exit(1);