thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH 0/5] labels: rework [Jul 2015]

If this information is useful, please help other people find it:
Share via:

Chen Hanxiao

2015-Jul-08 03:07 UTC

[Libguestfs] [PATCH 0/5] labels: rework

We should use the existing function from specific fs,
if not, move it to specific fs files.

Chen Hanxiao (5):
  label: move btrfslabel to btrfs.c
  label: move e2label to ext2.c and call it locally
  label: move ntfslabel to ntfs.c
  label: use existing do_xfs_admin for xfslabel
  labels: return ENOTSUP if could not set label for specific fs

 daemon/btrfs.c  | 16 +++++++++++
 daemon/daemon.h |  4 +++
 daemon/ext2.c   | 29 +++++++++++++++----
 daemon/labels.c | 89 ++++-----------------------------------------------------
 daemon/ntfs.c   | 19 ++++++++++++
 daemon/xfs.c    |  7 +++++
 6 files changed, 75 insertions(+), 89 deletions(-)

-- 
2.1.0

Chen Hanxiao

2015-Jul-08 03:07 UTC

head link

[Libguestfs] [PATCH 1/5] labels: move btrfslabel to btrfs.c

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/btrfs.c  | 16 ++++++++++++++++
 daemon/daemon.h |  1 +
 daemon/labels.c | 19 +------------------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 8fcfd81..ee3464d 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -70,6 +70,22 @@ btrfs_get_label (const char *device)
   return out;
 }
 
+int
+btrfs_set_label (const char *device, const char *label)
+{
+  int r;
+  CLEANUP_FREE char *err = NULL;
+
+  r = command (NULL, &err, str_btrfs, "filesystem",
"label",
+               device, label, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    return -1;
+  }
+
+  return 0;
+}
+
 /* Takes optional arguments, consult optargs_bitmask. */
 int
 do_btrfs_filesystem_resize (const char *filesystem, int64_t size)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index a4a4361..783d739 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -275,6 +275,7 @@ extern char *debug_bmap_device (const char *subcmd, size_t
argc, char *const *co
 
 /*-- in btrfs.c --*/
 extern char *btrfs_get_label (const char *device);
+extern int btrfs_set_label (const char *device, const char *label);
 extern int btrfs_set_uuid (const char *device, const char *uuid);
 extern int btrfs_set_uuid_random (const char *device);
 
diff --git a/daemon/labels.c b/daemon/labels.c
index cfcb4df..eec5b96 100644
--- a/daemon/labels.c
+++ b/daemon/labels.c
@@ -27,29 +27,12 @@
 #include "actions.h"
 #include "optgroups.h"
 
-GUESTFSD_EXT_CMD(str_btrfs, btrfs);
 GUESTFSD_EXT_CMD(str_dosfslabel, dosfslabel);
 GUESTFSD_EXT_CMD(str_e2label, e2label);
 GUESTFSD_EXT_CMD(str_ntfslabel, ntfslabel);
 GUESTFSD_EXT_CMD(str_xfs_admin, xfs_admin);
 
 static int
-btrfslabel (const char *device, const char *label)
-{
-  int r;
-  CLEANUP_FREE char *err = NULL;
-
-  r = command (NULL, &err, str_btrfs, "filesystem",
"label",
-               device, label, NULL);
-  if (r == -1) {
-    reply_with_error ("%s", err);
-    return -1;
-  }
-
-  return 0;
-}
-
-static int
 dosfslabel (const char *device, const char *label)
 {
   int r;
@@ -144,7 +127,7 @@ do_set_label (const mountable_t *mountable, const char
*label)
     return -1;
 
   if (STREQ (vfs_type, "btrfs"))
-    r = btrfslabel (mountable->device, label);
+    r = btrfs_set_label (mountable->device, label);
 
   else if (STREQ (vfs_type, "msdos") ||
            STREQ (vfs_type, "fat") ||
-- 
2.1.0

Chen Hanxiao

2015-Jul-08 03:07 UTC

head link

[Libguestfs] [PATCH 2/5] labels: move e2label to ext2.c and call it locally

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/daemon.h |  1 +
 daemon/ext2.c   | 29 +++++++++++++++++++++++------
 daemon/labels.c | 24 +-----------------------
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 783d739..0731b09 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -224,6 +224,7 @@ extern int sync_disks (void);
 #define EXT2_LABEL_MAX 16
 extern int fstype_is_extfs (const char *fstype);
 extern int ext_set_uuid_random (const char *device);
+extern int ext_set_label (const char *device, const char *label);
 
 /*-- in blkid.c --*/
 extern char *get_blkid_tag (const char *device, const char *tag);
diff --git a/daemon/ext2.c b/daemon/ext2.c
index e8d2655..7bb1221 100644
--- a/daemon/ext2.c
+++ b/daemon/ext2.c
@@ -38,6 +38,7 @@ GUESTFSD_EXT_CMD(str_resize2fs, resize2fs);
 GUESTFSD_EXT_CMD(str_mke2fs, mke2fs);
 GUESTFSD_EXT_CMD(str_lsattr, lsattr);
 GUESTFSD_EXT_CMD(str_chattr, chattr);
+GUESTFSD_EXT_CMD(str_e2label, e2label);
 
 /* https://bugzilla.redhat.com/show_bug.cgi?id=978302#c1 */
 int
@@ -123,14 +124,30 @@ do_tune2fs_l (const char *device)
 }
 
 int
+ext_set_label (const char *device, const char *label)
+{
+  int r;
+  CLEANUP_FREE char *err = NULL;
+
+  if (strlen (label) > EXT2_LABEL_MAX) {
+    reply_with_error ("%s: ext2 labels are limited to %d bytes",
+                      label, EXT2_LABEL_MAX);
+    return -1;
+  }
+
+  r = command (NULL, &err, str_e2label, device, label, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    return -1;
+  }
+
+  return 0;
+}
+
+int
 do_set_e2label (const char *device, const char *label)
 {
-  const mountable_t mountable = {
-    .type = MOUNTABLE_DEVICE,
-    .device = /* not really ... */ (char *) device,
-    .volume = NULL,
-  };
-  return do_set_label (&mountable, label);
+  return ext_set_label (device, label);
 }
 
 char *
diff --git a/daemon/labels.c b/daemon/labels.c
index eec5b96..1fadba4 100644
--- a/daemon/labels.c
+++ b/daemon/labels.c
@@ -28,7 +28,6 @@
 #include "optgroups.h"
 
 GUESTFSD_EXT_CMD(str_dosfslabel, dosfslabel);
-GUESTFSD_EXT_CMD(str_e2label, e2label);
 GUESTFSD_EXT_CMD(str_ntfslabel, ntfslabel);
 GUESTFSD_EXT_CMD(str_xfs_admin, xfs_admin);
 
@@ -48,27 +47,6 @@ dosfslabel (const char *device, const char *label)
 }
 
 static int
-e2label (const char *device, const char *label)
-{
-  int r;
-  CLEANUP_FREE char *err = NULL;
-
-  if (strlen (label) > EXT2_LABEL_MAX) {
-    reply_with_error ("%s: ext2 labels are limited to %d bytes",
-                      label, EXT2_LABEL_MAX);
-    return -1;
-  }
-
-  r = command (NULL, &err, str_e2label, device, label, NULL);
-  if (r == -1) {
-    reply_with_error ("%s", err);
-    return -1;
-  }
-
-  return 0;
-}
-
-static int
 ntfslabel (const char *device, const char *label)
 {
   int r;
@@ -135,7 +113,7 @@ do_set_label (const mountable_t *mountable, const char
*label)
     r = dosfslabel (mountable->device, label);
 
   else if (fstype_is_extfs (vfs_type))
-    r = e2label (mountable->device, label);
+    r = ext_set_label (mountable->device, label);
 
   else if (STREQ (vfs_type, "ntfs"))
     r = ntfslabel (mountable->device, label);
-- 
2.1.0

Chen Hanxiao

2015-Jul-08 03:07 UTC

head link

[Libguestfs] [PATCH 3/5] labels: move ntfslabel to ntfs.c

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/daemon.h |  1 +
 daemon/labels.c | 22 +---------------------
 daemon/ntfs.c   | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 0731b09..7a4b97f 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -282,6 +282,7 @@ extern int btrfs_set_uuid_random (const char *device);
 
 /*-- in ntfs.c --*/
 extern char *ntfs_get_label (const char *device);
+extern int ntfs_set_label (const char *device, const char *label);
 
 /*-- in swap.c --*/
 extern int swap_set_uuid (const char *device, const char *uuid);
diff --git a/daemon/labels.c b/daemon/labels.c
index 1fadba4..d3e3f55 100644
--- a/daemon/labels.c
+++ b/daemon/labels.c
@@ -28,7 +28,6 @@
 #include "optgroups.h"
 
 GUESTFSD_EXT_CMD(str_dosfslabel, dosfslabel);
-GUESTFSD_EXT_CMD(str_ntfslabel, ntfslabel);
 GUESTFSD_EXT_CMD(str_xfs_admin, xfs_admin);
 
 static int
@@ -47,25 +46,6 @@ dosfslabel (const char *device, const char *label)
 }
 
 static int
-ntfslabel (const char *device, const char *label)
-{
-  int r;
-  CLEANUP_FREE char *err = NULL;
-
-  /* XXX We should check if the label is longer than 128 unicode
-   * characters and return an error.  This is not so easy since we
-   * don't have the required libraries.
-   */
-  r = command (NULL, &err, str_ntfslabel, device, label, NULL);
-  if (r == -1) {
-    reply_with_error ("%s", err);
-    return -1;
-  }
-
-  return 0;
-}
-
-static int
 xfslabel (const char *device, const char *label)
 {
   int r;
@@ -116,7 +96,7 @@ do_set_label (const mountable_t *mountable, const char
*label)
     r = ext_set_label (mountable->device, label);
 
   else if (STREQ (vfs_type, "ntfs"))
-    r = ntfslabel (mountable->device, label);
+    r = ntfs_set_label (mountable->device, label);
 
   else if (STREQ (vfs_type, "xfs"))
     r = xfslabel (mountable->device, label);
diff --git a/daemon/ntfs.c b/daemon/ntfs.c
index 0f63391..1ead159 100644
--- a/daemon/ntfs.c
+++ b/daemon/ntfs.c
@@ -71,6 +71,25 @@ ntfs_get_label (const char *device)
 }
 
 int
+ntfs_set_label (const char *device, const char *label)
+{
+  int r;
+  CLEANUP_FREE char *err = NULL;
+
+  /* XXX We should check if the label is longer than 128 unicode
+   * characters and return an error.  This is not so easy since we
+   * don't have the required libraries.
+   */
+  r = command (NULL, &err, str_ntfslabel, device, label, NULL);
+  if (r == -1) {
+    reply_with_error ("%s", err);
+    return -1;
+  }
+
+  return 0;
+}
+
+int
 do_ntfs_3g_probe (int rw, const char *device)
 {
   CLEANUP_FREE char *err = NULL;
-- 
2.1.0

Chen Hanxiao

2015-Jul-08 03:07 UTC

head link

[Libguestfs] [PATCH 4/5] labels: use existing do_xfs_admin for xfslabel

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/daemon.h |  1 +
 daemon/labels.c | 18 +-----------------
 daemon/xfs.c    |  7 +++++++
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 7a4b97f..13add6c 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -268,6 +268,7 @@ extern int copy_xattrs (const char *src, const char *dest);
 #define XFS_LABEL_MAX 12
 extern int xfs_set_uuid (const char *device, const char *uuid);
 extern int xfs_set_uuid_random (const char *device);
+extern int xfs_set_label (const char *device, const char *label);
 
 /*-- debug-bmap.c --*/
 extern char *debug_bmap (const char *subcmd, size_t argc, char *const *const
argv);
diff --git a/daemon/labels.c b/daemon/labels.c
index d3e3f55..92d339c 100644
--- a/daemon/labels.c
+++ b/daemon/labels.c
@@ -28,7 +28,6 @@
 #include "optgroups.h"
 
 GUESTFSD_EXT_CMD(str_dosfslabel, dosfslabel);
-GUESTFSD_EXT_CMD(str_xfs_admin, xfs_admin);
 
 static int
 dosfslabel (const char *device, const char *label)
@@ -48,9 +47,6 @@ dosfslabel (const char *device, const char *label)
 static int
 xfslabel (const char *device, const char *label)
 {
-  int r;
-  CLEANUP_FREE char *err = NULL;
-
   /* Don't allow the special value "---".  If people want to
clear
    * the label we'll have to add another call to do that.
    */
@@ -59,19 +55,7 @@ xfslabel (const char *device, const char *label)
     return -1;
   }
 
-  if (strlen (label) > XFS_LABEL_MAX) {
-    reply_with_error ("%s: xfs labels are limited to %d bytes",
-                      label, XFS_LABEL_MAX);
-    return -1;
-  }
-
-  r = command (NULL, &err, str_xfs_admin, "-L", label, device,
NULL);
-  if (r == -1) {
-    reply_with_error ("%s", err);
-    return -1;
-  }
-
-  return 0;
+  return xfs_set_label (device, label);
 }
 
 int
diff --git a/daemon/xfs.c b/daemon/xfs.c
index 2c93311..e5e8b62 100644
--- a/daemon/xfs.c
+++ b/daemon/xfs.c
@@ -470,6 +470,13 @@ xfs_set_uuid_random (const char *device)
 }
 
 int
+xfs_set_label (const char *device, const char *label)
+{
+  optargs_bitmask = GUESTFS_XFS_ADMIN_LABEL_BITMASK;
+  return do_xfs_admin (device, 0, 0, 0, 0, 0, label, NULL);
+}
+
+int
 do_xfs_admin (const char *device,
               int extunwritten, int imgfile, int v2log,
               int projid32bit,
-- 
2.1.0

Chen Hanxiao

2015-Jul-08 03:07 UTC

head link

[Libguestfs] [PATCH 5/5] labels: return ENOTSUP if could not set label for specific fs

Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
---
 daemon/labels.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/daemon/labels.c b/daemon/labels.c
index 92d339c..a1a2f3b 100644
--- a/daemon/labels.c
+++ b/daemon/labels.c
@@ -85,11 +85,9 @@ do_set_label (const mountable_t *mountable, const char
*label)
   else if (STREQ (vfs_type, "xfs"))
     r = xfslabel (mountable->device, label);
 
-  else {
-    reply_with_error ("don't know how to set the label for
'%s' filesystems",
+  else
+    NOT_SUPPORTED(-1, "don't know how to set the label for
'%s' filesystems",
                       vfs_type);
-    r = -1;
-  }
 
   return r;
 }
-- 
2.1.0

Pino Toscano

2015-Jul-08 07:59 UTC

head link

Re: [Libguestfs] [PATCH 2/5] labels: move e2label to ext2.c and call it locally

Hi,

On Wednesday 08 July 2015 11:07:48 Chen Hanxiao wrote:>  int
> +ext_set_label (const char *device, const char *label)
> +{
> +  int r;
> +  CLEANUP_FREE char *err = NULL;
> +
> +  if (strlen (label) > EXT2_LABEL_MAX) {
> +    reply_with_error ("%s: ext2 labels are limited to %d bytes",
> +                      label, EXT2_LABEL_MAX);
> +    return -1;
> +  }
> +
> +  r = command (NULL, &err, str_e2label, device, label, NULL);
> +  if (r == -1) {
> +    reply_with_error ("%s", err);
> +    return -1;
> +  }
> +
> +  return 0;
> +}
I'd say you can just put all the implementation ...
> +int
>  do_set_e2label (const char *device, const char *label)
>  {
> -  const mountable_t mountable = {
> -    .type = MOUNTABLE_DEVICE,
> -    .device = /* not really ... */ (char *) device,
> -    .volume = NULL,
> -  };
> -  return do_set_label (&mountable, label);
> +  return ext_set_label (device, label);
>  }
... directly here, and just call do_set_e2label in do_set_label.

Thanks,
-- 
Pino Toscano

Pino Toscano

2015-Jul-08 08:01 UTC

head link

Re: [Libguestfs] [PATCH 5/5] labels: return ENOTSUP if could not set label for specific fs

Hi,

On Wednesday 08 July 2015 11:07:51 Chen Hanxiao wrote:> Signed-off-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com>
> ---
>  daemon/labels.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/daemon/labels.c b/daemon/labels.c
> index 92d339c..a1a2f3b 100644
> --- a/daemon/labels.c
> +++ b/daemon/labels.c
> @@ -85,11 +85,9 @@ do_set_label (const mountable_t *mountable, const char
*label)
>    else if (STREQ (vfs_type, "xfs"))
>      r = xfslabel (mountable->device, label);
>  
> -  else {
> -    reply_with_error ("don't know how to set the label for
'%s' filesystems",
> +  else
> +    NOT_SUPPORTED(-1, "don't know how to set the label for
'%s' filesystems",
>                        vfs_type);
> -    r = -1;
> -  }
>  
>    return r;
>  }
Can you please also amend the documentation for set_label, to mention
this error case?

Thanks,
-- 
Pino Toscano

Pino Toscano

2015-Jul-08 08:41 UTC

head link

Re: [Libguestfs] [PATCH 0/5] labels: rework

Hi,

On Wednesday 08 July 2015 11:07:46 Chen Hanxiao wrote:> We should use the existing function from specific fs,
> if not, move it to specific fs files.
> 
> Chen Hanxiao (5):
>   label: move btrfslabel to btrfs.c
>   label: move e2label to ext2.c and call it locally
>   label: move ntfslabel to ntfs.c
>   label: use existing do_xfs_admin for xfslabel
>   labels: return ENOTSUP if could not set label for specific fs
> 
>  daemon/btrfs.c  | 16 +++++++++++
>  daemon/daemon.h |  4 +++
>  daemon/ext2.c   | 29 +++++++++++++++----
>  daemon/labels.c | 89
++++-----------------------------------------------------
>  daemon/ntfs.c   | 19 ++++++++++++
>  daemon/xfs.c    |  7 +++++
>  6 files changed, 75 insertions(+), 89 deletions(-)
Nice work!

I've left easy comments on patches #2 and #5.
Patches #1, #3, and #4 are okay, but I could push only #1 because the
lack of #2 makes the others not apply.

Thanks,
-- 
Pino Toscano

Chen, Hanxiao

2015-Jul-08 09:03 UTC

head link

Re: [Libguestfs] [PATCH 0/5] labels: rework

> -----Original Message-----
> From: libguestfs-bounces@redhat.com [mailto:libguestfs-bounces@redhat.com]
On
> Behalf Of Pino Toscano
> Sent: Wednesday, July 08, 2015 4:42 PM
> To: libguestfs@redhat.com
> Subject: Re: [Libguestfs] [PATCH 0/5] labels: rework
> 
> Hi,
> 
> On Wednesday 08 July 2015 11:07:46 Chen Hanxiao wrote:
> > We should use the existing function from specific fs,
> > if not, move it to specific fs files.
> >
> > Chen Hanxiao (5):
> >   label: move btrfslabel to btrfs.c
> >   label: move e2label to ext2.c and call it locally
> >   label: move ntfslabel to ntfs.c
> >   label: use existing do_xfs_admin for xfslabel
> >   labels: return ENOTSUP if could not set label for specific fs
> >
> >  daemon/btrfs.c  | 16 +++++++++++
> >  daemon/daemon.h |  4 +++
> >  daemon/ext2.c   | 29 +++++++++++++++----
> >  daemon/labels.c | 89
> ++++-----------------------------------------------------
> >  daemon/ntfs.c   | 19 ++++++++++++
> >  daemon/xfs.c    |  7 +++++
> >  6 files changed, 75 insertions(+), 89 deletions(-)
> 
> Nice work!
> 
> I've left easy comments on patches #2 and #5.
> Patches #1, #3, and #4 are okay, but I could push only #1 because the
> lack of #2 makes the others not apply.
> 
I'll post v2 soon.

Regards,
- Chen

Possibly Parallel Threads

Search for more reasonably related threads

Libguestfs - Jul 2015 - Re: [PATCH 0/5] labels: rework

[Libguestfs] [PATCH 0/5] labels: rework

[Libguestfs] [PATCH 1/5] labels: move btrfslabel to btrfs.c

[Libguestfs] [PATCH 2/5] labels: move e2label to ext2.c and call it locally

[Libguestfs] [PATCH 3/5] labels: move ntfslabel to ntfs.c

[Libguestfs] [PATCH 4/5] labels: use existing do_xfs_admin for xfslabel

[Libguestfs] [PATCH 5/5] labels: return ENOTSUP if could not set label for specific fs

Re: [Libguestfs] [PATCH 2/5] labels: move e2label to ext2.c and call it locally

Re: [Libguestfs] [PATCH 5/5] labels: return ENOTSUP if could not set label for specific fs

Re: [Libguestfs] [PATCH 0/5] labels: rework

Re: [Libguestfs] [PATCH 0/5] labels: rework

Possibly Parallel Threads