Re: [Libguestfs] RFC: copy-attributes command
On Friday 10 January 2014 16:53:32 Richard W.M. Jones wrote:
On Fri, Jan 10, 2014 at 05:36:39PM +0100, Pino Toscano wrote:
[..]
This still isn't quite what I meant. My meaning was that mode
would be disabled by default (unless all:true).
OK.
How about:
int copy_mode, copy_xattributes, copy_ownership;
/* Set defaults. */
if (all)
copy_mode = copy_xattributes = copy_ownership = 1;
else
copy_mode = copy_xattributes = copy_ownership = 0;
/* If set in the original struct, copy those settings overriding
* the defaults.
*/
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
copy_mode = mode;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
copy_xattributes = xattributes;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
copy_ownership = ownership;
Isn't this doing basically the same of the snippet I used (the «if (all)
{ ... }» one), short of the part that enables mode if neither all nor
mode were specified?
--
Pino Toscano
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] RFC: copy-attributes command
On Mon, Jan 13, 2014 at 12:54:36PM +0100, Pino Toscano wrote:
On Friday 10 January 2014 16:53:32 Richard W.M. Jones wrote:
On Fri, Jan 10, 2014 at 05:36:39PM +0100, Pino Toscano wrote:
[..]
This still isn't quite what I meant. My meaning was that mode
would be disabled by default (unless all:true).
OK.
How about:
int copy_mode, copy_xattributes, copy_ownership;
/* Set defaults. */
if (all)
copy_mode = copy_xattributes = copy_ownership = 1;
else
copy_mode = copy_xattributes = copy_ownership = 0;
/* If set in the original struct, copy those settings overriding
* the defaults.
*/
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
copy_mode = mode;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
copy_xattributes = xattributes;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
copy_ownership = ownership;
Isn't this doing basically the same of the snippet I used (the «if (all)
{ ... }» one), short of the part that enables mode if neither all nor
mode were specified?
Probably. This way was just easier for me to understand :-)
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] RFC: copy-attributes command
On Tuesday 07 January 2014 21:04:36 Richard W.M. Jones wrote:
On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote:
Hi,
attached there is a prototype of patch for adding a new
copy-attributes command. Such command would allow copy the
attributes of a file to
another, so for example in guestfish:
copy-attributes foo bar permissions:true xattributes:false
would only copy the permissions of foo to bar, not copying its
extended attributes too.
I think the general idea of the new API is fine.
More comments about the code below.
Just few notes:
- my first daemon command, so possibly I could be missing something
- copy_xattrs is in xattr.c to avoid spreading the usage of xattr
API in
many places
- copy_xattrs does a bit of code repetition with other stuff in
xattr.c,
but I'm not sure how to avoid it without making the xattr listing
code (getxattrs) a bit more complex that what it is already
Comments?
diff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
/*-- in proto.c --*/
extern void main_loop (int sock) __attribute__((noreturn));
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
/* ordinary daemon functions use these to indicate errors
* NB: you don't need to prefix the string with the current
command,
* it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..fd6da71 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
#include guestfs_protocol.h
#include daemon.h
#include actions.h
+#include optgroups.h
GUESTFSD_EXT_CMD(str_file, file);
GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,46 @@ do_filesize (const char *path)
return buf.st_size;
}
+
+int
+do_copy_attributes (const char *src, const char *dest, int
permissions, int xattributes) +{
+ int r;
+ struct stat srcstat, deststat;
+
+ CHROOT_IN;
+ r = stat (src, srcstat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+ }
+
+ CHROOT_IN;
+ r = stat (dest, deststat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+ }
+
+ if (permissions ((srcstat.st_mode 0777) != (deststat.st_mode
0777))) { +CHROOT_IN;
+r = chmod (dest, (srcstat.st_mode 0777));
I suspect you want 0 in order to copy sticky/setuid/setgid bits.
Perhaps those should be a separate flag, but we definitely want to
copy them!
Right, fixed to be part of the permissions (or mode actually, see
below).
+ ssize_t len, vlen, ret;
+ CLEANUP_FREE char *buf = NULL, *attrval = NULL;
+ size_t i, attrval_len = 0;
+
+ CHROOT_IN;
+ len = listxattr (src, NULL, 0);
+ CHROOT_OUT;
+ if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+ }
+
+ buf = malloc (len);
+ if (buf == NULL) {
+reply_with_perror (malloc);
+goto error;
+ }
+
+ CHROOT_IN;
+ len = listxattr (src, buf, len);
+ CHROOT_OUT;
+ if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+ }
This two-pass snippet to do (l)listxattr is already elsewhere in
xattr.c, I will move it to an own function.
+ /* What we get from the kernel is a string foo\0bar\0baz of
length + * len.
+ */
+ for (i = 0; i (size_t) len; i += strlen (buf[i]) + 1) {
+CHROOT_IN;
+vlen = getxattr (src, buf[i], NULL, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+ reply_with_perror (getxattr: %s, %s, src, buf[i]);
+ goto error;
+}
+
+if (vlen XATTR_SIZE_MAX) {
+ /* The next call to getxattr will fail anyway, so ... */
+ reply_with_error (extended attribute is too large);
+ goto error;
+}
+
+if (vlen attrval_len) {
+ char *new = realloc (attrval, vlen);
+ if (new == NULL) {
+reply_with_perror (realloc);
+goto error;
+ }
+ attrval = new;
+ attrval_len = vlen;
+}
+
+CHROOT_IN;
+vlen = getxattr (src, buf[i], attrval, vlen);
+CHROOT_OUT;
+if (vlen == -1) {
+ reply_with_perror (getxattr: %s, %s, src, buf[i]);
+ goto error;
+}
+
+CHROOT_IN;
+ret = setxattr (dest, buf[i], attrval, vlen, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+ reply_with_perror (setxattr: %s, %s, dest, buf[i]);
+ goto error;
+}
+ }
This code looks as if it will copy the xattrs, but it won't
remove any which don't exist in the source. eg:
source xattrs before:
user.foo = 1
dest xattrs before:
user.bar = 2
dest xattrs after:
user.foo = 1
Re: [Libguestfs] RFC: copy-attributes command
On Fri, Jan 10, 2014 at 02:17:48PM +0100, Pino Toscano wrote: This code looks as if it will copy the xattrs, but it won't remove any which don't exist in the source. eg: source xattrs before: user.foo = 1 dest xattrs before: user.bar = 2 dest xattrs after: user.foo = 1 user.bar = 2 That may or may not be what we want, but I would say it's a bit unexpected. Yes, the current behaviour is done on purpose; my thought that, given the command is copy attributes, it would just copy what specified from the source to the destination. I see reasons for both the behaviours, so I'm not totally sure which one pick. On the basis that most users will be creating a new file (which will have no xattrs except in some very odd corner cases), just leave your implementation for now, but don't specify it in the documentation so we could change it later. - Should it default to copying attributes? You've convinced me, so I've turned the permissions flag into skipmode, so specifying nothing now copies the file mode (so permissions + bits). The common use case (for virt-edit) is: I want this other file which is identical to this original file, except in name and content. That is (I think) an argument for copying everything by default. I've added a all argument which would enable every change, overriding even mode:false. The API is now pretty confusing. Each OBool flag is really a tristate. It can either be true, false or not specified. Therefore I think it should be: xattributes:true# copies only xattrs and nothing else all:true# copies everything all:true xattributes:false # copies everything except xattrs In other words, 'all' changes the default (ie. not specified) state of the other flags. To be clearer, the four OBool parameters would be: [OBool all; OBool mode; OBool ownership; OBool xattributes] I would have thought owner uid and group gid should be copied. Oh true, forgot about them; added a new ownership argument. - Is there anything else which is useful to copy? Maybe there's also copying atime/mtime too; worth having a single argument (time) for both, or two separate? For virt-edit, I would _not_ want it to copy the old time(s). The time would be updated. But I guess there's a use for copying the time(s), so this could be another flag. We're allowed to extend the API later by adding optional flags (see Note about extending functions in generator/README for the complicated rules about extending APIs while preserving binary compatibility). +=over 4 + +=item Cxattributes + +Copy the Linux extended attributes (xattrs) from Csource to Cdestination. +This flag does nothing if the Ilinuxxattrs feature is not available +(see Cguestfs_feature_available). + +=item Call ^ Typo in this line. +Copy the owner uid and the group gid of Csource to Cdestination. + +=item Call + +Copy Ball the attributes from Csource to Cdestination. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] RFC: copy-attributes command
On Fri, Jan 10, 2014 at 01:33:38PM +, Richard W.M. Jones wrote: The API is now pretty confusing. Each OBool flag is really a tristate. It can either be true, false or not specified. Therefore I think it should be: xattributes:true# copies only xattrs and nothing else all:true# copies everything all:true xattributes:false # copies everything except xattrs In other words, 'all' changes the default (ie. not specified) state of the other flags. To be clearer, the four OBool parameters would be: [OBool all; OBool mode; OBool ownership; OBool xattributes] So looking at your v2 patch a bit more, I think this is what you already did, except that skipmode is backwards from the other flags. I think we're in agreement, except I think skipmode should just become mode and not have negated meaning compared to the other flags. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] RFC: copy-attributes command
On Friday 10 January 2014 13:33:38 Richard W.M. Jones wrote:
On Fri, Jan 10, 2014 at 02:17:48PM +0100, Pino Toscano wrote:
This code looks as if it will copy the xattrs, but it won't
remove any which don't exist in the source. eg:
source xattrs before:
user.foo = 1
dest xattrs before:
user.bar = 2
dest xattrs after:
user.foo = 1
user.bar = 2
That may or may not be what we want, but I would say it's a bit
unexpected.
Yes, the current behaviour is done on purpose; my thought that,
given
the command is copy attributes, it would just copy what specified
from the source to the destination.
I see reasons for both the behaviours, so I'm not totally sure which
one pick.
On the basis that most users will be creating a new file (which will
have no xattrs except in some very odd corner cases), just leave your
implementation for now, but don't specify it in the documentation so
we could change it later.
After all, the documentation says that it just copies the xattrs from
the source to the destination, but it does not explicitly mention what
is done with the xattrs in the destination not in the source ... :-)
On Friday 10 January 2014 13:36:06 Richard W.M. Jones wrote:
On Fri, Jan 10, 2014 at 01:33:38PM +, Richard W.M. Jones wrote:
The API is now pretty confusing. Each OBool flag is really a
tristate. It can either be true, false or not specified.
I see, I did not pay much attention to the use of optargs_bitmask
outside the auto-generated RPC stuff the daemon code.
Therefore I think it should be:
xattributes:true# copies only xattrs and nothing else
all:true# copies everything
all:true xattributes:false # copies everything except xattrs
In other words, 'all' changes the default (ie. not specified)
state
of the other flags.
Given the above, this indeed becomes straightforward to have.
To be clearer, the four OBool parameters would be:
[OBool all; OBool mode; OBool ownership; OBool
xattributes]
So looking at your v2 patch a bit more, I think this is what you
already did, except that skipmode is backwards from the other flags.
I think we're in agreement, except I think skipmode should just
become mode and not have negated meaning compared to the other
flags.
OK, I will turn it back as it was before (into mode), but making use
of the tristate information to have it true by default.
We're allowed to extend the API later by adding optional flags (see
Note about extending functions in generator/README for the
complicated rules about extending APIs while preserving binary
compatibility).
Yes, I read it earlier, that's why I'm not that concerned about adding
all the potential attributes now.
Attached there is the v3 of the patch.
--
Pino Toscanodiff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
/*-- in proto.c --*/
extern void main_loop (int sock) __attribute__((noreturn));
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
/* ordinary daemon functions use these to indicate errors
* NB: you don't need to prefix the string with the current command,
* it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..34ec95a 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
#include guestfs_protocol.h
#include daemon.h
#include actions.h
+#include optgroups.h
GUESTFSD_EXT_CMD(str_file, file);
GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,79 @@ do_filesize (const char *path)
return buf.st_size;
}
+
+int
+do_copy_attributes (const char *src, const char *dest, int all, int mode, int xattributes, int ownership)
+{
+ int r;
+ struct stat srcstat, deststat;
+
+ static const unsigned int file_mask = 0;
+
+ /* If it was specified to copy everything, manually enable all the flags
+ * not manually specified to avoid checking for flag || all everytime.
+ */
+ if (all) {
+if (!(optargs_bitmask GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
+ mode = 1;
+if (!(optargs_bitmask GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
+ xattributes = 1;
+if (!(optargs_bitmask GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
+ ownership = 1;
+ } else if ((!(optargs_bitmask GUESTFS_COPY_ATTRIBUTES_ALL_BITMASK))
+ (!(optargs_bitmask GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))) {
+/* Neither all nor mode were specified, so make mode on by default.
+ */
+mode = 1;
+ }
+
+ CHROOT_IN;
+ r = stat (src, srcstat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+ }
+
+ CHROOT_IN;
+ r = stat (dest, deststat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+ }
+
+ if (mode
+
Re: [Libguestfs] RFC: copy-attributes command
On Fri, Jan 10, 2014 at 05:36:39PM +0100, Pino Toscano wrote:
[..]
This still isn't quite what I meant. My meaning was that mode
would be disabled by default (unless all:true).
How about:
int copy_mode, copy_xattributes, copy_ownership;
/* Set defaults. */
if (all)
copy_mode = copy_xattributes = copy_ownership = 1;
else
copy_mode = copy_xattributes = copy_ownership = 0;
/* If set in the original struct, copy those settings overriding
* the defaults.
*/
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
copy_mode = mode;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
copy_xattributes = xattributes;
if ((optargs_bitmask GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
copy_ownership = ownership;
if (!copy_mode !copy_xattributes !copy_ownership) {
/* Short-circuit the code, but this *isn't* an error. */
return 0;
}
[...]
if (copy_mode)
// chmod
etc.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] RFC: copy-attributes command
On Tuesday 07 January 2014 21:04:36 Richard W.M. Jones wrote:
On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote:
diff --git a/daemon/xattr.c b/daemon/xattr.c
index af8bfd4..97a94d5 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -545,8 +545,98 @@ do_lgetxattr (const char *path, const char
*name, size_t *size_r)
return buf; /* caller frees */
}
+int
+copy_xattrs (const char *src, const char *dest)
+{
+#if defined(HAVE_LISTXATTR) defined(HAVE_GETXATTR)
defined(HAVE_SETXATTR)
I wonder if there are any platforms that lack one of listxattr,
getxattr and setxattr, but at the same time have one of these calls.
The xattr code (in general) is incredibly complex because of all these
tests.
I guess Mac OS X probably has none of them, and RHEL 5 / Ubuntu 10.10
probably had only some of them.
At least in the gnu/stubs-$bits.h of RHEL 5 they are not declared as
__stub_*, so they should be implemented (and being Linux, it means that
most probably there are the right syscalls for them).
So I would not think the change proposed below (and implemented) would
cause regressions in such old Linux systems.
We don't care about RHEL 5 since it
now has its own branch (oldlinux), so the code might be made simpler
by an accompanying patch which reduces all of the HAVE_*XATTR* macros
down to a single one (HAVE_LINUX_XATTRS).
Sounds reasonable; attached a patch for it.
--
Pino ToscanoFrom fc4dd8cab10b1c3571c49c0ed8bd3de29e98bd6c Mon Sep 17 00:00:00 2001
From: Pino Toscano ptosc...@redhat.com
Date: Thu, 9 Jan 2014 17:21:31 +0100
Subject: [PATCH] daemon: xattr: simplify the enabling of the linuxxattrs
features
Instead of enable them when having one of the two headers for it but
still checking for the HAVE_* availability of each *xattr() function
used, just enable the linuxxattrs as a whole when having any of the
needed headers (like before) and all the needed functions.
This might cause the linuxxattrs to not be available anymore on systems
without the whole set of *xattr() functions implemented, but OTOH it
simplifies the xattr.c implementations.
---
daemon/xattr.c | 49 +++--
1 file changed, 11 insertions(+), 38 deletions(-)
diff --git a/daemon/xattr.c b/daemon/xattr.c
index af8bfd4..b84cf3d 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -27,7 +27,15 @@
#include actions.h
#include optgroups.h
-#if defined(HAVE_ATTR_XATTR_H) || defined(HAVE_SYS_XATTR_H)
+#if (defined(HAVE_ATTR_XATTR_H) || defined(HAVE_SYS_XATTR_H)) \
+defined(HAVE_LISTXATTR) defined(HAVE_LLISTXATTR) \
+defined(HAVE_GETXATTR) defined(HAVE_LGETXATTR) \
+defined(HAVE_REMOVEXATTR) defined(HAVE_LREMOVEXATTR) \
+defined(HAVE_SETXATTR) defined(HAVE_LSETXATTR)
+# define HAVE_LINUX_XATTRS
+#endif
+
+#ifdef HAVE_LINUX_XATTRS
# ifdef HAVE_ATTR_XATTR_H
# include attr/xattr.h
@@ -50,67 +58,37 @@ static int _removexattr (const char *xattr, const char *path, int (*removexattr)
guestfs_int_xattr_list *
do_getxattrs (const char *path)
{
-#if defined(HAVE_LISTXATTR) defined(HAVE_GETXATTR)
return getxattrs (path, listxattr, getxattr);
-#else
- reply_with_error (no support for listxattr and getxattr);
- return NULL;
-#endif
}
guestfs_int_xattr_list *
do_lgetxattrs (const char *path)
{
-#if defined(HAVE_LLISTXATTR) defined(HAVE_LGETXATTR)
return getxattrs (path, llistxattr, lgetxattr);
-#else
- reply_with_error (no support for llistxattr and lgetxattr);
- return NULL;
-#endif
}
int
do_setxattr (const char *xattr, const char *val, int vallen, const char *path)
{
-#if defined(HAVE_SETXATTR)
return _setxattr (xattr, val, vallen, path, setxattr);
-#else
- reply_with_error (no support for setxattr);
- return -1;
-#endif
}
int
do_lsetxattr (const char *xattr, const char *val, int vallen, const char *path)
{
-#if defined(HAVE_LSETXATTR)
return _setxattr (xattr, val, vallen, path, lsetxattr);
-#else
- reply_with_error (no support for lsetxattr);
- return -1;
-#endif
}
int
do_removexattr (const char *xattr, const char *path)
{
-#if defined(HAVE_REMOVEXATTR)
return _removexattr (xattr, path, removexattr);
-#else
- reply_with_error (no support for removexattr);
- return -1;
-#endif
}
int
do_lremovexattr (const char *xattr, const char *path)
{
-#if defined(HAVE_LREMOVEXATTR)
return _removexattr (xattr, path, lremovexattr);
-#else
- reply_with_error (no support for lremovexattr);
- return -1;
-#endif
}
static int
@@ -277,7 +255,6 @@ _removexattr (const char *xattr, const char *path,
guestfs_int_xattr_list *
do_internal_lxattrlist (const char *path, char *const *names)
{
-#if defined(HAVE_LLISTXATTR) defined(HAVE_LGETXATTR)
guestfs_int_xattr_list *ret = NULL;
size_t i, j;
size_t k, m, nr_attrs;
@@ -443,10 +420,6 @@ do_internal_lxattrlist (const char *path, char *const *names)
free (ret);
}
return NULL;
-#else
- reply_with_error (no support for
Re: [Libguestfs] RFC: copy-attributes command
On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote:
Hi,
attached there is a prototype of patch for adding a new copy-attributes
command. Such command would allow copy the attributes of a file to
another, so for example in guestfish:
copy-attributes foo bar permissions:true xattributes:false
would only copy the permissions of foo to bar, not copying its extended
attributes too.
I think the general idea of the new API is fine.
More comments about the code below.
Just few notes:
- my first daemon command, so possibly I could be missing something
- copy_xattrs is in xattr.c to avoid spreading the usage of xattr API in
many places
- copy_xattrs does a bit of code repetition with other stuff in xattr.c,
but I'm not sure how to avoid it without making the xattr listing code
(getxattrs) a bit more complex that what it is already
Comments?
--
Pino Toscano
diff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
/*-- in proto.c --*/
extern void main_loop (int sock) __attribute__((noreturn));
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
/* ordinary daemon functions use these to indicate errors
* NB: you don't need to prefix the string with the current command,
* it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..fd6da71 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
#include guestfs_protocol.h
#include daemon.h
#include actions.h
+#include optgroups.h
GUESTFSD_EXT_CMD(str_file, file);
GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,46 @@ do_filesize (const char *path)
return buf.st_size;
}
+
+int
+do_copy_attributes (const char *src, const char *dest, int permissions, int
xattributes)
+{
+ int r;
+ struct stat srcstat, deststat;
+
+ CHROOT_IN;
+ r = stat (src, srcstat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+ }
+
+ CHROOT_IN;
+ r = stat (dest, deststat);
+ CHROOT_OUT;
+
+ if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+ }
+
+ if (permissions ((srcstat.st_mode 0777) != (deststat.st_mode
0777))) {
+CHROOT_IN;
+r = chmod (dest, (srcstat.st_mode 0777));
I suspect you want 0 in order to copy sticky/setuid/setgid bits.
Perhaps those should be a separate flag, but we definitely want to
copy them!
+CHROOT_OUT;
+
+if (r == -1) {
+ reply_with_perror (chmod: %s, dest);
+ return -1;
+}
+ }
+
+ if (xattributes optgroup_linuxxattrs_available ()) {
+if (!copy_xattrs (src, dest))
+ return -1;
+ }
+
+ return 0;
+}
diff --git a/daemon/xattr.c b/daemon/xattr.c
index af8bfd4..97a94d5 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -545,8 +545,98 @@ do_lgetxattr (const char *path, const char *name, size_t
*size_r)
return buf; /* caller frees */
}
+int
+copy_xattrs (const char *src, const char *dest)
+{
+#if defined(HAVE_LISTXATTR) defined(HAVE_GETXATTR)
defined(HAVE_SETXATTR)
I wonder if there are any platforms that lack one of listxattr,
getxattr and setxattr, but at the same time have one of these calls.
The xattr code (in general) is incredibly complex because of all these
tests.
I guess Mac OS X probably has none of them, and RHEL 5 / Ubuntu 10.10
probably had only some of them. We don't care about RHEL 5 since it
now has its own branch (oldlinux), so the code might be made simpler
by an accompanying patch which reduces all of the HAVE_*XATTR* macros
down to a single one (HAVE_LINUX_XATTRS).
+ ssize_t len, vlen, ret;
+ CLEANUP_FREE char *buf = NULL, *attrval = NULL;
+ size_t i, attrval_len = 0;
+
+ CHROOT_IN;
+ len = listxattr (src, NULL, 0);
+ CHROOT_OUT;
+ if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+ }
+
+ buf = malloc (len);
+ if (buf == NULL) {
+reply_with_perror (malloc);
+goto error;
+ }
+
+ CHROOT_IN;
+ len = listxattr (src, buf, len);
+ CHROOT_OUT;
+ if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+ }
+
+ /* What we get from the kernel is a string foo\0bar\0baz of length
+ * len.
+ */
+ for (i = 0; i (size_t) len; i += strlen (buf[i]) + 1) {
+CHROOT_IN;
+vlen = getxattr (src, buf[i], NULL, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+ reply_with_perror (getxattr: %s, %s, src, buf[i]);
+ goto error;
+}
+
+if (vlen XATTR_SIZE_MAX) {
+ /* The next call to getxattr will fail anyway, so ... */
+ reply_with_error (extended attribute is too large);
+ goto error;
+}
+
+if (vlen attrval_len) {
+ char *new = realloc (attrval, vlen);
+ if (new == NULL) {
+
Re: [Libguestfs] RFC: copy-attributes command
On Tue, Jan 07, 2014 at 09:04:36PM +, Richard W.M. Jones wrote: - Should it default to copying attributes? The common use case (for virt-edit) is: I want this other file which is identical to this original file, except in name and content. That is (I think) an argument for copying everything by default. Also there's no reason we can't have another bool optarg to select whether the other bool optargs should default to true or false ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/ ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
