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 +0000, 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 Toscano
Attachments:
- copy-attributes-3.diff (text/x-patch — 9.8 KB)