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
[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
[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
[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
[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
[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
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
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
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
> -----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