Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
On Tue, Nov 01, 2016 at 09:10:09AM +0800, Qu Wenruo wrote: > I'm just a little curious about the stream samples. > Should I create a send-dump-test to include all these samples? Or put > them somewhere else? Misc tests would be fine for now, as you did in the new patchset. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
At 11/01/2016 01:31 AM, David Sterba wrote:
On Wed, Oct 19, 2016 at 09:18:09AM +0800, Qu Wenruo wrote:
At 10/18/2016 08:07 PM, David Sterba wrote:
On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
Hi David,
Any updates?
If you're busy on these fix, I could fix these problem and send a new
version.
I was about to add the series to merge but the output format needs work.
At minimum, something that's parseable, so some commonly used format
(yaml or json). The current way is ok for quick overview, but I'd really
like to see more options of the output. The malloc/free problems are
minor.
Thanks for the comment.
It makes sense. And it can be the first step to make all btrfs commands
output parseable.
I could try to use json-c as our json library to add json output mode
for send dump.
Meanwhile the original design for send-dump is only for human, and the
json output support will be a huge work(not only for send-dump, but
quite a lot of other sub-commands).
What about merge send dump first, and then add json support step-by-step?
Ok, so let's do it in another way.
The output format:
* one line per stream operation
* use key=value for all operation data
* file paths are unquoted, but escaped (same what seq_escape(" \t\n\\") does)
* two options where to put the filename, first or last, let's keep it
first for now, we can move it later on,
for multiple paths per operation, one of them will be just anoter
key=value with descriptive key name
Code changes:
* fold my two patches from branch dev/send-dump-wip
* the common block at the beginniing of each callback can be factored
into a macro (ie. full_path, ret, and path_cat with the name of the
operation)
We'll also need some stream samples that trigger all operations so we
can tune the output.
This sounds quite good to me.
I'll update the patchset.
I'm just a little curious about the stream samples.
Should I create a send-dump-test to include all these samples? Or put
them somewhere else?
Thanks,
Qu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
On Wed, Oct 19, 2016 at 09:18:09AM +0800, Qu Wenruo wrote:
> At 10/18/2016 08:07 PM, David Sterba wrote:
> > On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
> >> Hi David,
> >>
> >> Any updates?
> >>
> >> If you're busy on these fix, I could fix these problem and send a new
> >> version.
> >
> > I was about to add the series to merge but the output format needs work.
> > At minimum, something that's parseable, so some commonly used format
> > (yaml or json). The current way is ok for quick overview, but I'd really
> > like to see more options of the output. The malloc/free problems are
> > minor.
>
> Thanks for the comment.
>
> It makes sense. And it can be the first step to make all btrfs commands
> output parseable.
>
> I could try to use json-c as our json library to add json output mode
> for send dump.
>
> Meanwhile the original design for send-dump is only for human, and the
> json output support will be a huge work(not only for send-dump, but
> quite a lot of other sub-commands).
>
> What about merge send dump first, and then add json support step-by-step?
Ok, so let's do it in another way.
The output format:
* one line per stream operation
* use key=value for all operation data
* file paths are unquoted, but escaped (same what seq_escape(" \t\n\\") does)
* two options where to put the filename, first or last, let's keep it
first for now, we can move it later on,
for multiple paths per operation, one of them will be just anoter
key=value with descriptive key name
Code changes:
* fold my two patches from branch dev/send-dump-wip
* the common block at the beginniing of each callback can be factored
into a macro (ie. full_path, ret, and path_cat with the name of the
operation)
We'll also need some stream samples that trigger all operations so we
can tune the output.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
At 10/18/2016 08:07 PM, David Sterba wrote: On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote: Hi David, Any updates? If you're busy on these fix, I could fix these problem and send a new version. I was about to add the series to merge but the output format needs work. At minimum, something that's parseable, so some commonly used format (yaml or json). The current way is ok for quick overview, but I'd really like to see more options of the output. The malloc/free problems are minor. Thanks for the comment. It makes sense. And it can be the first step to make all btrfs commands output parseable. I could try to use json-c as our json library to add json output mode for send dump. Meanwhile the original design for send-dump is only for human, and the json output support will be a huge work(not only for send-dump, but quite a lot of other sub-commands). What about merge send dump first, and then add json support step-by-step? Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote: > Hi David, > > Any updates? > > If you're busy on these fix, I could fix these problem and send a new > version. I was about to add the series to merge but the output format needs work. At minimum, something that's parseable, so some commonly used format (yaml or json). The current way is ok for quick overview, but I'd really like to see more options of the output. The malloc/free problems are minor. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
Hi David,
Any updates?
If you're busy on these fix, I could fix these problem and send a new
version.
Thanks,
Qu
At 09/08/2016 05:56 PM, David Sterba wrote:
On Thu, Sep 08, 2016 at 11:42:29AM +0200, David Sterba wrote:
On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
@@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
}
}
- ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
+ if (dump) {
+ struct btrfs_dump_send_args dump_args;
+
+ dump_args.root_path = malloc(PATH_MAX);
+ dump_args.root_path[0] = '.';
+ dump_args.root_path[1] = '\0';
+ dump_args.full_subvol_path = malloc(PATH_MAX);
Please always check malloc return values. I'm fixing this for now.
Uh and the buffers are not freed either. Anyway, I'm switching it to an
array, there's no reason to allocate the memory dynamically.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
On Thu, Sep 08, 2016 at 11:42:29AM +0200, David Sterba wrote:
> On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
> > @@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
> > }
> > }
> >
> > - ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> > + if (dump) {
> > + struct btrfs_dump_send_args dump_args;
> > +
> > + dump_args.root_path = malloc(PATH_MAX);
> > + dump_args.root_path[0] = '.';
> > + dump_args.root_path[1] = '\0';
> > + dump_args.full_subvol_path = malloc(PATH_MAX);
>
> Please always check malloc return values. I'm fixing this for now.
Uh and the buffers are not freed either. Anyway, I'm switching it to an
array, there's no reason to allocate the memory dynamically.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream
On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
> @@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
> }
> }
>
> - ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> + if (dump) {
> + struct btrfs_dump_send_args dump_args;
> +
> + dump_args.root_path = malloc(PATH_MAX);
> + dump_args.root_path[0] = '.';
> + dump_args.root_path[1] = '\0';
> + dump_args.full_subvol_path = malloc(PATH_MAX);
Please always check malloc return values. I'm fixing this for now.
> + dump_args.full_subvol_path[0] = '.';
> + dump_args.full_subvol_path[1] = '\0';
> + ret = btrfs_read_and_process_send_stream(receive_fd,
> + &btrfs_print_send_ops, &dump_args, 0, 0);
> + if (ret < 0)
> + error("failed to dump the send stream: %s",
> + strerror(-ret));
> + } else {
> + ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> + }
> +
> if (receive_fd != fileno(stdin))
> close(receive_fd);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
