[Libguestfs] [PATCH] builder, edit, fish: use copy-attributes
Richard W.M. Jones
rjones at redhat.com
Tue Jan 14 12:07:50 UTC 2014
On Tue, Jan 14, 2014 at 11:16:40AM +0100, Pino Toscano wrote:
> Make use of the new copy-attributes command to properly copy all file
> attributes from a file to the new version of it.
> ---
> builder/perl_edit.ml | 29 +---------------------------
> edit/edit.c | 49 ++---------------------------------------------
> fish/edit.c | 54 ++--------------------------------------------------
> 3 files changed, 5 insertions(+), 127 deletions(-)
>
> diff --git a/builder/perl_edit.ml b/builder/perl_edit.ml
> index aa4c2e6..28e5dea 100644
> --- a/builder/perl_edit.ml
> +++ b/builder/perl_edit.ml
> @@ -42,7 +42,7 @@ let rec edit_file ~debug (g : Guestfs.guestfs) file expr =
> g#upload tmpfile file;
>
> (* However like virt-edit we do need to copy attributes. *)
> - copy_attributes g file_old file;
> + g#copy_attributes ~all:true file_old file;
> g#rm file_old
>
> and do_perl_edit ~debug g file expr =
> @@ -76,30 +76,3 @@ and do_perl_edit ~debug g file expr =
> );
>
> Unix.rename (file ^ ".out") file
> -
> -and copy_attributes g src dest =
> - let has_linuxxattrs = g#feature_available [|"linuxxattrs"|] in
> -
> - (* Get the mode. *)
> - let stat = g#stat src in
> -
> - (* Get the SELinux context. XXX Should we copy over other extended
> - * attributes too?
> - *)
> - let selinux_context =
> - if has_linuxxattrs then (
> - try Some (g#getxattr src "security.selinux") with _ -> None
> - ) else None in
> -
> - (* Set the permissions (inc. sticky and set*id bits), UID, GID. *)
> - let mode = Int64.to_int stat.G.mode
> - and uid = Int64.to_int stat.G.uid and gid = Int64.to_int stat.G.gid in
> - g#chmod (mode land 0o7777) dest;
> - g#chown uid gid dest;
> -
> - (* Set the SELinux context. *)
> - match selinux_context with
> - | None -> ()
> - | Some selinux_context ->
> - g#setxattr "security.selinux"
> - selinux_context (String.length selinux_context) dest
> diff --git a/edit/edit.c b/edit/edit.c
> index e5e3a29..07790be 100644
> --- a/edit/edit.c
> +++ b/edit/edit.c
> @@ -56,7 +56,6 @@ static void edit_files (int argc, char *argv[]);
> static void edit (const char *filename, const char *root);
> static char *edit_interactively (const char *tmpfile);
> static char *edit_non_interactively (const char *tmpfile);
> -static int copy_attributes (const char *src, const char *dest);
> static int is_windows (guestfs_h *g, const char *root);
> static char *windows_path (guestfs_h *g, const char *root, const char *filename);
> static char *generate_random_name (const char *filename);
> @@ -361,7 +360,8 @@ edit (const char *filename, const char *root)
> /* Set the permissions, UID, GID and SELinux context of the new
> * file to match the old file (RHBZ#788641).
> */
> - if (copy_attributes (filename, newname) == -1)
> + if (guestfs_copy_attributes (g, filename, newname,
> + GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1)
> goto error;
>
> /* Backup or overwrite the file. */
> @@ -510,51 +510,6 @@ edit_non_interactively (const char *tmpfile)
> }
>
> static int
> -copy_attributes (const char *src, const char *dest)
> -{
> - CLEANUP_FREE_STAT struct guestfs_stat *stat = NULL;
> - const char *linuxxattrs[] = { "linuxxattrs", NULL };
> - int has_linuxxattrs;
> - CLEANUP_FREE char *selinux_context = NULL;
> - size_t selinux_context_size;
> -
> - has_linuxxattrs = guestfs_feature_available (g, (char **) linuxxattrs);
> -
> - /* Get the mode. */
> - stat = guestfs_stat (g, src);
> - if (stat == NULL)
> - return -1;
> -
> - /* Get the SELinux context. XXX Should we copy over other extended
> - * attributes too?
> - */
> - if (has_linuxxattrs) {
> - guestfs_push_error_handler (g, NULL, NULL);
> -
> - selinux_context = guestfs_getxattr (g, src, "security.selinux",
> - &selinux_context_size);
> - /* selinux_context could be NULL. This isn't an error. */
> -
> - guestfs_pop_error_handler (g);
> - }
> -
> - /* Set the permissions (inc. sticky and set*id bits), UID, GID. */
> - if (guestfs_chmod (g, stat->mode & 07777, dest) == -1)
> - return -1;
> - if (guestfs_chown (g, stat->uid, stat->gid, dest) == -1)
> - return -1;
> -
> - /* Set the SELinux context. */
> - if (has_linuxxattrs && selinux_context) {
> - if (guestfs_setxattr (g, "security.selinux", selinux_context,
> - (int) selinux_context_size, dest) == -1)
> - return -1;
> - }
> -
> - return 0;
> -}
> -
> -static int
> is_windows (guestfs_h *g, const char *root)
> {
> int w;
> diff --git a/fish/edit.c b/fish/edit.c
> index 754a34a..bd02f4b 100644
> --- a/fish/edit.c
> +++ b/fish/edit.c
> @@ -32,7 +32,6 @@
> #include "fish.h"
>
> static char *generate_random_name (const char *filename);
> -static int copy_attributes (const char *src, const char *dest);
>
> /* guestfish edit command, suggested by Ján Ondrej, implemented by RWMJ */
>
> @@ -135,7 +134,8 @@ run_edit (const char *cmd, size_t argc, char *argv[])
> /* Set the permissions, UID, GID and SELinux context of the new
> * file to match the old file (RHBZ#788641).
> */
> - if (copy_attributes (remotefilename, newname) == -1)
> + if (guestfs_copy_attributes (g, remotefilename, newname,
> + GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1)
> return -1;
>
> if (guestfs_mv (g, newname, remotefilename) == -1)
> @@ -177,53 +177,3 @@ generate_random_name (const char *filename)
>
> return ret; /* caller will free */
> }
> -
> -static int
> -copy_attributes (const char *src, const char *dest)
> -{
> - struct guestfs_stat *stat;
> - const char *linuxxattrs[] = { "linuxxattrs", NULL };
> - int has_linuxxattrs;
> - CLEANUP_FREE char *selinux_context = NULL;
> - size_t selinux_context_size;
> -
> - has_linuxxattrs = guestfs_feature_available (g, (char **) linuxxattrs);
> -
> - /* Get the mode. */
> - stat = guestfs_stat (g, src);
> - if (stat == NULL)
> - return -1;
> -
> - /* Get the SELinux context. XXX Should we copy over other extended
> - * attributes too?
> - */
> - if (has_linuxxattrs) {
> - guestfs_push_error_handler (g, NULL, NULL);
> -
> - selinux_context = guestfs_getxattr (g, src, "security.selinux",
> - &selinux_context_size);
> - /* selinux_context could be NULL. This isn't an error. */
> -
> - guestfs_pop_error_handler (g);
> - }
> -
> - /* Set the permissions (inc. sticky and set*id bits), UID, GID. */
> - if (guestfs_chmod (g, stat->mode & 07777, dest) == -1) {
> - guestfs_free_stat (stat);
> - return -1;
> - }
> - if (guestfs_chown (g, stat->uid, stat->gid, dest) == -1) {
> - guestfs_free_stat (stat);
> - return -1;
> - }
> - guestfs_free_stat (stat);
> -
> - /* Set the SELinux context. */
> - if (has_linuxxattrs && selinux_context) {
> - if (guestfs_setxattr (g, "security.selinux", selinux_context,
> - (int) selinux_context_size, dest) == -1)
> - return -1;
> - }
> -
> - return 0;
> -}
> --
> 1.8.3.1
Looks good, ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
More information about the Libguestfs
mailing list