Hi all,
The attached patch implements a dd clone that can perform I to/from CIFS
servers. Samba 4 folks, could you please review this before I check it in?

Tue, 2006-01-17 at 13:16 +1100, James Peach wrote:
Hi all,

The attached patch implements a dd clone that can perform I to/from CIFS
servers. Samba 4 folks, could you please review this before I check it in?

How critical is standard DD syntax? It seems a real pity to avoid the
standard popt we provide for the rest of Samba4.

This would avoid your credentials problem. (Supporting the SMB url would
be nice, but it sucks for conveying username, domain and password
info).

Also, it ensures you correctly setup debug, loadparm and many other
systems. We hook our 'must be done by every samba program' code into
popt, because it's called at the 'right time'.

coding style, it just doesn't seem to match that in which the rest of
Samba4 is written. Now, this hasn't been a hard and fast rule in
Samba3, but in Samba4's prog_guide.txt the Linux kernel style has been
specified. (I mostly noticed the type-before-function stuff).

This does look like an interesting utility, and I hope we can work this
stuff out.

Andrew Bartlett

Tue, 17 Jan 2006 01:39 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 13:16 +1100, James Peach wrote:
Hi all,

The attached patch implements a dd clone that can perform I to/from CIFS
servers. Samba 4 folks, could you please review this before I check it
in?

How critical is standard DD syntax? It seems a real pity to avoid the
standard popt we provide for the rest of Samba4.

Without dd syntax it wouldn't be a dd clone, which would defeat (some of)
the purpose from my point of view.

This would avoid your credentials problem. (Supporting the SMB url would
be nice, but it sucks for conveying username, domain and password
info).

Also, it ensures you correctly setup debug, loadparm and many other
systems. We hook our 'must be done by every samba program' code into
popt, because it's called at the 'right time'.

Is there something I missed?

coding style, it just doesn't seem to match that in which the rest of
Samba4 is written. Now, this hasn't been a hard and fast rule in
Samba3, but in Samba4's prog_guide.txt the Linux kernel style has been
specified. (I mostly noticed the type-before-function stuff).

K, I've tidied up the function definitions. Anything else?

Tue, 2006-01-17 at 14:46 +1100, James Peach wrote:
Tue, 17 Jan 2006 01:39 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 13:16 +1100, James Peach wrote:
Hi all,

The attached patch implements a dd clone that can perform I to/from CIFS
servers. Samba 4 folks, could you please review this before I check it
in?

How critical is standard DD syntax? It seems a real pity to avoid the
standard popt we provide for the rest of Samba4.

Without dd syntax it wouldn't be a dd clone, which would defeat (some of)
the purpose from my point of view.

Even for all the 'extra' stuff like authentication? I would be very sad
to see this go in without hooking on the common credentials, version and
config handling.

I suppose I just don't see the benefit in being a perfect dd clone, over
the costs of being different.

This would avoid your credentials problem. (Supporting the SMB url would
be nice, but it sucks for conveying username, domain and password
info).

Also, it ensures you correctly setup debug, loadparm and many other
systems. We hook our 'must be done by every samba program' code into
popt, because it's called at the 'right time'.

Is there something I missed?

I'm not sure, but if you saw quite how much pain went into ensuring that
every Samba3 binary called load_case_tables(), I can see this utility
suffering from bitrot.

You should certainly ensure it features in 'make test'.

coding style, it just doesn't seem to match that in which the rest of
Samba4 is written. Now, this hasn't been a hard and fast rule in
Samba3, but in Samba4's prog_guide.txt the Linux kernel style has been
specified. (I mostly noticed the type-before-function stuff).

K, I've tidied up the function definitions. Anything else?

Mostly it just looked 'different', but I couldn't pin it down to
anything else.

Andrew Bartlett

Tue, 17 Jan 2006 03:06 pm, tridge (AT) samba (DOT) org wrote:
James,

Without dd syntax it wouldn't be a dd clone, which would defeat (some
of) the purpose from my point of view.

Can you explain why it being a clone is important? Could your aims be
met by making it a smbclient command and you using a small wrapper
script?

It's designed as a testing tool. We (and presumably others) have scripts
that know how to drive dd so the command line interface is convenient. I
go to some lengths to make sure that the I sizes seen on the wire are
those that are requested on the command line.

Having code like sdd.c in Samba that re-invents popt, and avoids using
all the existing command line handling code for options (especially
auth options) is something that I'm very reluctant to do, so it would
require some very strong justifications!

I guess I don't feel as strongly about this. I expect to use this for
generating client load. Strictly speaking I don't need _any_ authentication.

Maybe I could add the popt handling prior to looking at any dd arguments?
sdd [common samba options] [dd-style options]

Tue, 17 Jan 2006 03:08 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 14:46 +1100, James Peach wrote:
Tue, 17 Jan 2006 01:39 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 13:16 +1100, James Peach wrote:
Hi all,

The attached patch implements a dd clone that can perform I to/from
CIFS servers. Samba 4 folks, could you please review this before I
check it in?

How critical is standard DD syntax? It seems a real pity to avoid the
standard popt we provide for the rest of Samba4.

Without dd syntax it wouldn't be a dd clone, which would defeat (some of)
the purpose from my point of view.

Even for all the 'extra' stuff like authentication? I would be very sad
to see this go in without hooking on the common credentials, version and
config handling.

I suppose I just don't see the benefit in being a perfect dd clone, over
the costs of being different.

This would avoid your credentials problem. (Supporting the SMB url
would be nice, but it sucks for conveying username, domain and password
info).

Also, it ensures you correctly setup debug, loadparm and many other
systems. We hook our 'must be done by every samba program' code into
popt, because it's called at the 'right time'.

Is there something I missed?

I'm not sure, but if you saw quite how much pain went into ensuring that
every Samba3 binary called load_case_tables(), I can see this utility
suffering from bitrot.

You should certainly ensure it features in 'make test'.

K. I'd like to do this is a subsequent checkin, however.

coding style, it just doesn't seem to match that in which the rest
of Samba4 is written. Now, this hasn't been a hard and fast rule in
Samba3, but in Samba4's prog_guide.txt the Linux kernel style has been
specified. (I mostly noticed the type-before-function stuff).

K, I've tidied up the function definitions. Anything else?

Mostly it just looked 'different', but I couldn't pin it down to
anything else.

Well, I'm happy to fix anything once you tell me what it is :)

Tue, 17 Jan 2006 03:16 pm, James Peach wrote:
Tue, 17 Jan 2006 03:06 pm, tridge (AT) samba (DOT) org wrote:
[snip]
Having code like sdd.c in Samba that re-invents popt, and avoids using
all the existing command line handling code for options (especially
auth options) is something that I'm very reluctant to do, so it would
require some very strong justifications!

I guess I don't feel as strongly about this. I expect to use this for
generating client load. Strictly speaking I don't need _any_
authentication.

Maybe I could add the popt handling prior to looking at any dd arguments?
sdd [common samba options] [dd-style options]

The attached patch implements this

Tue, 2006-01-17 at 16:18 +1100, James Peach wrote:
Tue, 17 Jan 2006 03:16 pm, James Peach wrote:
Tue, 17 Jan 2006 03:06 pm, tridge (AT) samba (DOT) org wrote:
[snip]
Having code like sdd.c in Samba that re-invents popt, and avoids using
all the existing command line handling code for options (especially
auth options) is something that I'm very reluctant to do, so it would
require some very strong justifications!

I guess I don't feel as strongly about this. I expect to use this for
generating client load. Strictly speaking I don't need _any_
authentication.

Maybe I could add the popt handling prior to looking at any dd arguments?
sdd [common samba options] [dd-style options]

The attached patch implements this

You still don't use cmdline_credentials. This global variable is what
the PPT_CMMN_CREDENTIALS creates.

I think tridge has a point about making this an smbclient command.
this is all done, what will be the difference between the operation of
the modified sdd and smbclient -c 'dd '?

Why the change to popt.h?

Andrew Bartlett

Tue, 17 Jan 2006 09:26 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 16:18 +1100, James Peach wrote:
Tue, 17 Jan 2006 03:16 pm, James Peach wrote:
Tue, 17 Jan 2006 03:06 pm, tridge (AT) samba (DOT) org wrote:

[snip]

Having code like sdd.c in Samba that re-invents popt, and avoids
using all the existing command line handling code for options
(especially auth options) is something that I'm very reluctant to do,
so it would require some very strong justifications!

I guess I don't feel as strongly about this. I expect to use this for
generating client load. Strictly speaking I don't need _any_
authentication.

Maybe I could add the popt handling prior to looking at any dd
arguments? sdd [common samba options] [dd-style options]

The attached patch implements this

You still don't use cmdline_credentials. This global variable is what
the PPT_CMMN_CREDENTIALS creates.

, I didn't understand that this was what you meant. IIRC, the reason I
wanted to use the SMB URL for the "if" and "of" options is the case where
you have 2 connections do different servers and you need to have different
authentication information for each. This will probably be a rare case,
however, so I can certainly use cmdline_credentials until it's needed.

the issue raised on IRC of there already being a dd clone called
"sdd", I'm more than happy to change the name. cifsdd?

I think tridge has a point about making this an smbclient command.
this is all done, what will be the difference between the operation of
the modified sdd and smbclient -c 'dd '?

First, the actual syntax of smbclient is smbclient -c 'foo' //host/share. This
creates ambiguity in the dd syntax.

Where does the data go in this case?
smbclient -c 'dd if=/dir/file1 of=/dev/null" //server/share

What about this case?
smbclient -c 'dd if=//server1/share/dir/file1
of=/dir/file1" //server/share

The dd subcommand certainly wouldn't operate like any of the other smbclient
subcommands.

Second, the smbclient syntax is incompatible with existing test harnesses that
drive dd. The usage I'm planning for this is something like:
DD=/usr/samba/bin/sdd SRC=//server/share/foo DST=/dev/null ./test.sh

Third, the sdd command is carefully constructed to do exactly the I
you tell it to as far as possible. If you specify a 32KiB block size,
then you get a 32KiB read on the wire. To move this code into smbclient
would require moving its I path. smbclient would then contain 2 parallell
I paths. I don't see how making smbclient more complicated in this way
is helpful.

Why the change to popt.h?

There was a gcc warning; when returning const char * const, the second const
has no effect.

Wed, 2006-01-18 at 10:12 +1100, James Peach wrote:
Tue, 17 Jan 2006 09:26 pm, Andrew Bartlett wrote:
Tue, 2006-01-17 at 16:18 +1100, James Peach wrote:
Tue, 17 Jan 2006 03:16 pm, James Peach wrote:
Tue, 17 Jan 2006 03:06 pm, tridge (AT) samba (DOT) org wrote:

[snip]

Having code like sdd.c in Samba that re-invents popt, and avoids
using all the existing command line handling code for options
(especially auth options) is something that I'm very reluctant to do,
so it would require some very strong justifications!

I guess I don't feel as strongly about this. I expect to use this for
generating client load. Strictly speaking I don't need _any_
authentication.

Maybe I could add the popt handling prior to looking at any dd
arguments? sdd [common samba options] [dd-style options]

The attached patch implements this

You still don't use cmdline_credentials. This global variable is what
the PPT_CMMN_CREDENTIALS creates.

, I didn't understand that this was what you meant. IIRC, the reason I
wanted to use the SMB URL for the "if" and "of" options is the case where
you have 2 connections do different servers and you need to have different
authentication information for each. This will probably be a rare case,
however, so I can certainly use cmdline_credentials until it's needed.

If or when a change away from cmdline_credentials is made, it should be
fixed for both this tool and for some of the smbtorture commands, which
would also like multiple sets of authentication credentials.

However, I would warn you that this is actually quite hard to do right
(and easy to do wrong) at the command line level. we are in C code
it is of course easy, now we have those structures.

the issue raised on IRC of there already being a dd clone called
"sdd", I'm more than happy to change the name. cifsdd?

I think tridge has a point about making this an smbclient command.
this is all done, what will be the difference between the operation of
the modified sdd and smbclient -c 'dd '?

First, the actual syntax of smbclient is smbclient -c 'foo' //host/share. This
creates ambiguity in the dd syntax.

Where does the data go in this case?
smbclient -c 'dd if=/dir/file1 of=/dev/null" //server/share

What about this case?
smbclient -c 'dd if=//server1/share/dir/file1
of=/dir/file1" //server/share

The dd subcommand certainly wouldn't operate like any of the other smbclient
subcommands.

Perhaps we need to change the syntax a bit more (then do any munging in
a shell wrapper, to better match what you want in for your test harness)

smbclient -c 'blockwrite <local<remote<options>'
smbclient -c 'blockread <remote<local<options>'

The wrapper script can then pull apart which has the //server/share/
prefix, and call the appropriate subcommand.

Second, the smbclient syntax is incompatible with existing test harnesses that
drive dd. The usage I'm planning for this is something like:
DD=/usr/samba/bin/sdd SRC=//server/share/foo DST=/dev/null ./test.sh

Third, the sdd command is carefully constructed to do exactly the I
you tell it to as far as possible. If you specify a 32KiB block size,
then you get a 32KiB read on the wire. To move this code into smbclient
would require moving its I path. smbclient would then contain 2 parallell
I paths. I don't see how making smbclient more complicated in this way
is helpful.

smbcacls was a tool created to handle a particular task in Samba, and no
doubt did that task quite well, without disrupting other tools. It also
broke very often, and was unmaintained and untested for long periods of
time.

The situation is better these days, particularly thanks to the popt,
better documentation (magic includes of common options) and other
tricks, but there is still a cost.

I'm sorry to be so discouraging.

Why the change to popt.h?

There was a gcc warning; when returning const char * const, the second const
has no effect.

I would prefer to leave it intact. That code is meant to be simply the
imported popt project. We should update it sometime.

Andrew Bartlett

Wed, 18 Jan 2006 10:44 am, Andrew Bartlett wrote:
Wed, 2006-01-18 at 10:12 +1100, James Peach wrote:
[snip]
The dd subcommand certainly wouldn't operate like any of the other smbclient
subcommands.

I would go further, and say that the only thing (cif)sdd has in common
with smbclient is that they both connect to an CIFS server. They have
entirely different purposes and audiences (and therefore different syntax
and implementation).

Perhaps we need to change the syntax a bit more (then do any munging in
a shell wrapper, to better match what you want in for your test harness)

I imagine that there are a lot of scripts out there that know how to
drive smbclient. I'm not sure that changing its syntax is a good idea.

smbclient -c 'blockwrite <local<remote<options>'
smbclient -c 'blockread <remote<local<options>'

Do you mean
smbclient //server1/foo -c 'blockwrite <local<remote<options>'
smbclient //server1/foo -c 'blockread <remote<local<options>'

Does <remoterefer to server1 a file on server1, or can it specify a
different server? What does this look like then I want to do
cifsdd if=//server/foo/source of=//server1/foo/dest

The dd syntax above is unambigious. The block{read,write} syntax is
making my head hurt.

The wrapper script can then pull apart which has the //server/share/
prefix, and call the appropriate subcommand.

In this case, I have to maintain the blockwrite code path in smbclient
as well as the wrapper. This seems very likely to break.

Second, the smbclient syntax is incompatible with existing test harnesses that
drive dd. The usage I'm planning for this is something like:
DD=/usr/samba/bin/sdd SRC=//server/share/foo DST=/dev/null ./test.sh

Third, the sdd command is carefully constructed to do exactly the I
you tell it to as far as possible. If you specify a 32KiB block size,
then you get a 32KiB read on the wire. To move this code into smbclient
would require moving its I path. smbclient would then contain 2 parallell
I paths. I don't see how making smbclient more complicated in this way
is helpful.

smbcacls was a tool created to handle a particular task in Samba, and no
doubt did that task quite well, without disrupting other tools. It also
broke very often, and was unmaintained and untested for long periods of
time.

If smbcacls bitrotted, that's because it was unmaintained and
unused. that is a risk for any piece of code irrespective of
its location in the source tree. Since I intend to use this tool, then
it's reasonable to assume it will continue to work. Adding a test for
it into "make test" will ensure that at least the basics will continue
to work in my absence.

Is the team position really that smbclient is the only client? How
do we intend to ensure that none of the 50 (and presumably growing)
subcommands in smbclient bitrots? I assume that people outside the team
are going to want to write CIFS clients for various reasons and that
this will be supported.

The situation is better these days, particularly thanks to the popt,
better documentation (magic includes of common options) and other
tricks, but there is still a cost.

I'm sorry to be so discouraging.

I'm less discouraged than puzzled. Shoving all sorts of unrelated gunk
into smbclient seems to me to be a recipe for producing unmaintainable
code. AFAICT the only objection is that separate commands are less
maintainable, but this is a straw man IM

Why the change to popt.h?

There was a gcc warning; when returning const char * const, the second const
has no effect.

I would prefer to leave it intact. That code is meant to be simply the
imported popt project. We should update it sometime.

ok.