[Libguestfs] [PATCH 2/3] api: add mountable_device and mountable_subvolume
Pino Toscano
ptoscano at redhat.com
Tue Mar 1 12:16:41 UTC 2016
On Tuesday 01 March 2016 11:17:18 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 | 25 +++++++++++++++++++++++++
> po/POTFILES | 1 +
> src/Makefile.am | 1 +
> src/mountable.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 75 insertions(+)
> create mode 100644 src/mountable.c
>
> diff --git a/generator/actions.ml b/generator/actions.ml
> index eb45392..454164f 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -1290,6 +1290,31 @@ Please read L<guestfs(3)/INSPECTION> for more details.
> See also C<guestfs_inspect_get_mountpoints>." };
>
> { defaults with
> + name = "mountable_device"; added = (1, 33, 13);
1.33.13 is tagged now, so this should be 1.33.14; it is not a big deal
though, as it will be adjusted on the fly when applying the commit by
whoever (Rich or me) is pushing it.
> + 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)." };
I'd split the longdesc in two paragraph, the first describing what the
API does, and the second about the case with btrfs.
I'm not a native English speaker though.
> + { defaults with
> + name = "mountable_subvolume"; added = (1, 33, 13);
> + 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 has no subvolume part, then the errno (see
> +C<guestfs_last_errno>) is set to C<EINVAL>." };
I'd say something like "If the mountable does not represent a btrfs
subvolume, then this function fails and the errno is set to 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 0fb99b0..4912f9f 100644
> --- a/po/POTFILES
> +++ b/po/POTFILES
> @@ -345,6 +345,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..8f27606
> --- /dev/null
> +++ b/src/mountable.c
> @@ -0,0 +1,48 @@
> +/* 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 <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;
> +
> + if ((mnt = guestfs_internal_parse_mountable (g, mountable)))
> + return safe_strdup(g, mnt->im_device);
Avoid side-effects in if statements, and fail earlier:
mnt = guestfs_internal_parse_mountable (g, mountable);
if (mnt == NULL)
return NULL;
return safe_strdup (g, mnt->im_device);
> +
> + return NULL;
> +}
> +
> +char *
> +guestfs_impl_mountable_subvolume (guestfs_h *g, const char *mountable)
> +{
> + CLEANUP_FREE_INTERNAL_MOUNTABLE struct guestfs_internal_mountable *mnt = NULL;
> +
> + if ((mnt = guestfs_internal_parse_mountable (g, mountable)))
> + return safe_strdup(g, mnt->im_volume);
Ditto.
> + errno = EINVAL;
libguestfs' errno is not the system one -- there's a proper internal
API for this, see the various error/warning functions/macros in
src/guestfs-internal.h. In this case, you need to use
guestfs_int_error_errno explicitly:
guestfs_int_error_errno (g, EINVAL, "not a btrfs subvolume identifier");
Also, note that in case of non-btrfs mountables,
guestfs_internal_parse_mountable succeeds but im_volume will be an
empty string, so you need to consider that.
Thanks,
--
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160301/408cee74/attachment.sig>
More information about the Libguestfs
mailing list