[PATCH v3 0/3] btrfs subvolumes display fix
Hi there, Latest and greatest version including all new remarks from both Pino and Rich. Cédric Bosdonnat (3): configure: handle older version of ncurses api: add mountable_device and mountable_subvolume fish: fix btrfs subvolumes display in error case fish/options.c | 28 ++++++++++++++++++++++++++- generator/actions.ml | 26 +++++++++++++++++++++++++ m4/guestfs_libraries.m4 | 9 ++++++++- po/POTFILES | 1 + src/Makefile.am | 1 + src/mountable.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 src/mountable.c -- 2.6.2
ncurses didn't have pkg-config files in not-that-old versions. If those couldn't be found, then try the ncurses6-config and ncurses5-config tools. --- m4/guestfs_libraries.m4 | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4 index c5a4a01..e845ea9 100644 --- a/m4/guestfs_libraries.m4 +++ b/m4/guestfs_libraries.m4 @@ -113,7 +113,14 @@ struct sockaddr_un myaddr; dnl tgetent, tputs and UP [sic] are all required. They come from the lower dnl tinfo library, but might be part of ncurses directly. PKG_CHECK_MODULES([LIBTINFO], [tinfo], [], [ - PKG_CHECK_MODULES([LIBTINFO], [ncurses]) + PKG_CHECK_MODULES([LIBTINFO], [ncurses], [], [ + AC_CHECK_PROGS([NCURSES_CONFIG], [ncurses6-config ncurses5-config], [no]) + AS_IF([test "x$NCURSES_CONFIG" = "xno"], [ + AC_MSG_ERROR([ncurses development package is not installed]) + ]) + LIBTINFO_CFLAGS=`$NCURSES_CONFIG --cflags` + LIBTINFO_LIBS=`$NCURSES_CONFIG --libs` + ]) ]) AC_SUBST([LIBTINFO_CFLAGS]) AC_SUBST([LIBTINFO_LIBS]) -- 2.6.2
These two functions allow the user to split the mountable strings into a device and a subvolume if any. See this thread on the mailing list for the rationale: https://www.redhat.com/archives/libguestfs/2016-February/msg00247.html --- generator/actions.ml | 26 ++++++++++++++++++++++++++ po/POTFILES | 1 + src/Makefile.am | 1 + src/mountable.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+) create mode 100644 src/mountable.c diff --git a/generator/actions.ml b/generator/actions.ml index 36d08ad..51f2295 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1290,6 +1290,32 @@ Please read L<guestfs(3)/INSPECTION> for more details. See also C<guestfs_inspect_get_mountpoints>." }; { defaults with + name = "mountable_device"; added = (1, 33, 15); + style = RString "device", [Mountable "mountable"], []; + shortdesc = "extract the device part of a mountable"; + longdesc = "\ +Returns the device name of a mountable. In quite a lot of +cases, the mountable is the device name. + +However this doesn't apply for btrfs subvolumes, where the +mountable is a combination of both the device name and the +subvolume path (see also C<guestfs_mountable_subvolume> to +extract the subvolume path of the mountable if any)." }; + + { defaults with + name = "mountable_subvolume"; added = (1, 33, 15); + style = RString "subvolume", [Mountable "mountable"], []; + shortdesc = "extract the subvolume part of a mountable"; + longdesc = "\ +Returns the subvolume path of a mountable. Btrfs subvolumes +mountables are a combination of both the device name and the +subvolume path (see also C<guestfs_mountable_device> to extract +the device of the mountable). + +If the mountable does not represent a btrfs subvolume, then +this function fails and the C<errno> is set to C<EINVAL>." }; + + { defaults with name = "set_network"; added = (1, 5, 4); style = RErr, [Bool "network"], []; fish_alias = ["network"]; config_only = true; diff --git a/po/POTFILES b/po/POTFILES index 4fbc551..195206f 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -346,6 +346,7 @@ src/libvirt-is-version.c src/listfs.c src/lpj.c src/match.c +src/mountable.c src/osinfo.c src/private-data.c src/proto.c diff --git a/src/Makefile.am b/src/Makefile.am index 60641bf..3b4cd10 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -121,6 +121,7 @@ libguestfs_la_SOURCES = \ listfs.c \ lpj.c \ match.c \ + mountable.c \ osinfo.c \ private-data.c \ proto.c \ diff --git a/src/mountable.c b/src/mountable.c new file mode 100644 index 0000000..fb982df --- /dev/null +++ b/src/mountable.c @@ -0,0 +1,51 @@ +/* libguestfs + * Copyright (C) 2016 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> +#include <string.h> +#include <errno.h> + +#include "guestfs.h" +#include "guestfs-internal.h" + + +char * +guestfs_impl_mountable_device (guestfs_h *g, const char *mountable) +{ + CLEANUP_FREE_INTERNAL_MOUNTABLE struct guestfs_internal_mountable *mnt = NULL; + + mnt = guestfs_internal_parse_mountable (g, mountable); + if (mnt == NULL) + return NULL; + + return safe_strdup (g, mnt->im_device); +} + +char * +guestfs_impl_mountable_subvolume (guestfs_h *g, const char *mountable) +{ + CLEANUP_FREE_INTERNAL_MOUNTABLE struct guestfs_internal_mountable *mnt = NULL; + + mnt = guestfs_internal_parse_mountable (g, mountable); + if (mnt == NULL || STREQ (mnt->im_volume, "")) { + guestfs_int_error_errno (g, EINVAL, "not a btrfs subvolume identifier"); + return NULL; + } + + return safe_strdup (g, mnt->im_volume); +} -- 2.6.2
On Tuesday 08 March 2016 16:04:21 Cédric Bosdonnat wrote:
These two functions allow the user to split the mountable strings into a device and a subvolume if any. See this thread on the mailing list for the rationale:
https://www.redhat.com/archives/libguestfs/2016-February/msg00247.html --- generator/actions.ml | 26 ++++++++++++++++++++++++++ po/POTFILES | 1 + src/Makefile.am | 1 + src/mountable.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+) create mode 100644 src/mountable.c
diff --git a/generator/actions.ml b/generator/actions.ml index 36d08ad..51f2295 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1290,6 +1290,32 @@ Please read L<guestfs(3)/INSPECTION> for more details. See also C<guestfs_inspect_get_mountpoints>." };
{ defaults with + name = "mountable_device"; added = (1, 33, 15); + style = RString "device", [Mountable "mountable"], []; + shortdesc = "extract the device part of a mountable"; + longdesc = "\ +Returns the device name of a mountable. In quite a lot of +cases, the mountable is the device name. + +However this doesn't apply for btrfs subvolumes, where the +mountable is a combination of both the device name and the +subvolume path (see also C<guestfs_mountable_subvolume> to +extract the subvolume path of the mountable if any)." }; + + { defaults with + name = "mountable_subvolume"; added = (1, 33, 15); + style = RString "subvolume", [Mountable "mountable"], []; + shortdesc = "extract the subvolume part of a mountable"; + longdesc = "\ +Returns the subvolume path of a mountable. Btrfs subvolumes +mountables are a combination of both the device name and the +subvolume path (see also C<guestfs_mountable_device> to extract +the device of the mountable). + +If the mountable does not represent a btrfs subvolume, then +this function fails and the C<errno> is set to C<EINVAL>." }; + + { defaults with name = "set_network"; added = (1, 5, 4); style = RErr, [Bool "network"], []; fish_alias = ["network"]; config_only = true; diff --git a/po/POTFILES b/po/POTFILES index 4fbc551..195206f 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -346,6 +346,7 @@ src/libvirt-is-version.c src/listfs.c src/lpj.c src/match.c +src/mountable.c src/osinfo.c src/private-data.c src/proto.c diff --git a/src/Makefile.am b/src/Makefile.am index 60641bf..3b4cd10 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -121,6 +121,7 @@ libguestfs_la_SOURCES = \ listfs.c \ lpj.c \ match.c \ + mountable.c \ osinfo.c \ private-data.c \ proto.c \ diff --git a/src/mountable.c b/src/mountable.c new file mode 100644 index 0000000..fb982df --- /dev/null +++ b/src/mountable.c @@ -0,0 +1,51 @@ +/* libguestfs + * Copyright (C) 2016 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> +#include <string.h> +#include <errno.h> + +#include "guestfs.h" +#include "guestfs-internal.h" +
I'm noticing now that the guestfs-internal-actions.h include is missing here, and thus the functions here will not build because of missing prototypes. I'm fixing manually and pushing, no need to send a new version of this patch now. My suggestion is to configure libguestfs for development with --enable-werror, so warnings are turned into errors (and thus are harder to miss). Thanks, -- Pino Toscano
The list of filesystems that is printed when there was an error prints the internal mountable string even for the btrfs subvolumes. Let's printing a valid -m option value instead. --- fish/options.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/fish/options.c b/fish/options.c index da5015d..0dffff5 100644 --- a/fish/options.c +++ b/fish/options.c @@ -280,7 +280,33 @@ display_mountpoints_on_failure (const char *mp_device, guestfs_int_program_name); for (i = 0; fses[i] != NULL; i += 2) { - CLEANUP_FREE char *p = guestfs_canonical_device_name (g, fses[i]); + CLEANUP_FREE char *p = NULL; + CLEANUP_FREE char *device = guestfs_mountable_device (g, fses[i]); + CLEANUP_FREE char *subvolume = NULL; + + guestfs_push_error_handler (g, NULL, NULL); + + subvolume = guestfs_mountable_subvolume (g, fses[i]); + if (subvolume == NULL && guestfs_last_errno (g) != EINVAL) { + fprintf (stderr, + _("%s: cannot determine the subvolume for %s: %s (%d)\n"), + guestfs_int_program_name, fses[i], + guestfs_last_error (g), guestfs_last_errno (g)); + exit (EXIT_FAILURE); + } + + guestfs_pop_error_handler (g); + + /* Reformat the internal btrfsvol string into a valid mount option */ + if (device && subvolume) { + if (asprintf (&p, "%s:/:subvol=%s", device, subvolume) == -1) { + perror ("asprintf"); + exit (EXIT_FAILURE); + } + } else { + p = guestfs_canonical_device_name (g, fses[i]); + } + fprintf (stderr, "%s: \t%s (%s)\n", guestfs_int_program_name, p ? p : fses[i], fses[i+1]); } -- 2.6.2
On Tuesday 08 March 2016 16:04:19 Cédric Bosdonnat wrote:
Latest and greatest version including all new remarks from both Pino and Rich.
Cédric Bosdonnat (3): configure: handle older version of ncurses api: add mountable_device and mountable_subvolume fish: fix btrfs subvolumes display in error case
fish/options.c | 28 ++++++++++++++++++++++++++- generator/actions.ml | 26 +++++++++++++++++++++++++ m4/guestfs_libraries.m4 | 9 ++++++++- po/POTFILES | 1 + src/Makefile.am | 1 + src/mountable.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 src/mountable.c
Just with a small amend in patch #2, series tested fine and pushed. Thanks, -- Pino Toscano
participants (2)
-
Cédric Bosdonnat -
Pino Toscano