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
Possibly Parallel Threads
- [PATCH] daemon: add a space after func/macro to fit code-style
- [PATCH 0/5] labels: rework
- [PATCH 3/5] labels: move ntfslabel to ntfs.c
- [PATCH] uuids: return ENOTSUP if could not set UUID for specific fs
- [PATCH 1/2] daemon: allow to change the labels of swap partitions