Bug #541

multi-vkd support patch for review

Added by c.turner about 7 years ago. Updated about 7 years ago.

Status:ClosedStart date:
Priority:LowDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

Well.. I suppose it's time to stop lurking and come out of the
woodwork..

Attached is a multi-vkd patch for vkernels on the 1.8-RELEASE branch.
(plus a couple of very minor typo fixups in the multi-vke code)

Since I'm pretty out of date on my C, no comments can be too harsh :)

a.k.a

I'm not used to strong typing at the moment, unless you count 'cons'
or 'my %tab;' a strong type..

so I'll be glad to fix asap - hoping something like this can
make release, although I know it might be too late..

The patch tries to match the multi-vke organization as much as it can,
maxing out via #define at 16 vkd's, which should be pretty good
for most purposes. Also, it kludges the '-r' argument to mean
'disk' - let me know if I should fix the manpage / vkernel
usage statement..

As far as testing goes, I basically made sure I can label, newfs,
mount and fill the disks.. vinum/ccd remains incomplete/unested..
(see below)

Simple FFS Check:

Fill up a bunch of single-partition disks..

# uname -sr
DragonFly 1.8.0-RELEASE
# df
Filesystem 1K-blocks Used Avail Capacity Mounted on
/dev/vkd0a 1032142 314428 635144 33% /
/dev/vkd1a 120926 120898 -9646 109% /mnt/1
/dev/vkd2a 120926 120898 -9646 109% /mnt/2
/dev/vkd3a 120926 120898 -9646 109% /mnt/3
/dev/vkd4a 120926 120898 -9646 109% /mnt/4
/dev/vkd5a 120926 120898 -9646 109% /mnt/5
/dev/vkd6a 120926 120898 -9646 109% /mnt/6
/dev/vkd7a 120926 120898 -9646 109% /mnt/7
/dev/vkd8a 120926 120898 -9646 109% /mnt/8
/dev/vkd9a 120926 120898 -9646 109% /mnt/9
/dev/vkd10a 120926 120898 -9646 109% /mnt/10
/dev/vkd11a 120926 120898 -9646 109% /mnt/11
/dev/vkd12a 120926 120898 -9646 109% /mnt/12
/dev/vkd13a 120926 120898 -9646 109% /mnt/13
/dev/vkd14a 120926 120898 -9646 109% /mnt/14
/dev/vkd15a 120926 120898 -9646 109% /mnt/15
# du -sk /mnt/*/*
120896 /mnt/1/testfile
120896 /mnt/10/testfile
120896 /mnt/11/testfile
120896 /mnt/12/testfile
120896 /mnt/13/testfile
120896 /mnt/14/testfile
120896 /mnt/15/testfile
238224 /mnt/16/testfile
120896 /mnt/2/testfile
120896 /mnt/3/testfile
120896 /mnt/4/testfile
120896 /mnt/5/testfile
120896 /mnt/6/testfile
120896 /mnt/7/testfile
120896 /mnt/8/testfile
120896 /mnt/9/testfile
# uptime
12:09AM up 42 mins, 1 user, load averages: 6.30, 9.58, 5.45

(/mnt/16/ results from my 'test accessing one too many disks' logic
gone bad)

Vinum Check:

well - perhaps eventually.. current status (after relabeling):

# cat vinum.conf.disks
drive d1 device /dev/vkd1a
drive d2 device /dev/vkd2a
# vinum create -f vinum.conf.disks
Bus error (core dumped)

and I think I won't look further as the build box is a PII-450
that's chewing on a pgksrc bulk at the moment..

Lurker Comments:

I've been using DragonFly since late 1.4.x as my day-to-day desktop,
and I must say the experience is great, I like all the goals, pkgsrc
is a great fit, and I'm looking forward to using the VKERNEL for more
systems testing (hopefully with a faster box :)..
thanks to the team for running a great project!

I've got some other vkernel / dfly related ideas floating around
in my head, that I might work on but would like to throw out there
while I'm on my lurk-box to test the waters in case I don't get
to them just yet:

- arbitrary VKERNEL tty's via a fifo for 'detached' operation
- some kind of control socket protocol
(e.g. host-initiated clean shutdown, etc),
or signal handler hook-scripts, etc.
- leaving a pidfile somehwere
- auto host-side pf rulesets on vkernel startup
(to constrain the vkernels)
- expanding vkd's into some kind of spoofed cam / scsi bus
to allow for 'hotswap' vkds , etc.
- importing KAME SCTP, if there's any use for it and I'm capable of
it.. (that one just got in my head somehow.. *cough* syslink
transport ..?? ;)

Any comments on implementing some of the above would be welcome -
my kernel skillz are very weak. Need to get the McKusic book methinks :)
Probably the tty-on-a-pipe would be my first choice / next target..

Thanks again for creating a great system!

- Chris

multi-vkd.patch Magnifier (7.35 KB) c.turner, 01/28/2007 02:08 AM

History

#1 Updated by TGEN about 7 years ago

Some people might argue that's what GNU screen is for, and fifos might
not be the ideal way (plus, they're not ttys).

This is possible, there are already many programs (on OpenBSD) hooking
in to pf (e.g., openbgpd).

CAM isn't particularly nice though.

We already have SCTP support in-tree :).

Why not if (vkdFileNum < VKD_MAX) { vkdFile[vkdFileNum] = optarg;
vkdFileNum++; }; ? Because now it appears '16' is a valid number, where
you start counting from 0, meaning a maximum of 17 devices...

Cheers,
--
Thomas E. Spanjaard

#2 Updated by sepherosa about 7 years ago

Why '16'(VKD_MAX) will be a valid number in his patch?

Best Regards,
sephe

#3 Updated by c.turner about 7 years ago

On Sun, 28 Jan 2007 13:01:36 +0000
"Thomas E. Spanjaard" <> wrote:

did some more thinking and this was bad word choice -
Yes - screen has crossed my mind, and probably what I'll use
in the meantime.. but screen has problems too in some cases
(think ":screen /bin/sh" ) pty would be better, agreed.

Basically, something (exact "something" tbd) to provide (controlled,
secure) access to in-vkernel 'local' tty's outside of the parent
process of the vkernel..

exactly!

oops.. missed that one.. well.. never mind.

Basically this was a presumptuous way of asking if it had been thought
of as a potential transport for syslink, more than anything else :)

I basically cloned this from the multi-vke code in the above switch..

(had something else, then I looked at that and would be more consistent,
although I kind of wondered about postincrement here as well..)

But yes.. it on closer inspection it looks like the order of
operations is out of whack here and above..

the checks in init_vkd below, and in vdisk.c:vkdinit() should catch it
anyway, but bugs should be squashed!

Like I said:

"Since I'm pretty out of date on my C, no comments can be too harsh :)"

Let me know how to proceed
(since this bug seems to be present in other code as well)

Thanks,

- Chris

#4 Updated by TGEN about 7 years ago

if (15 < 16) { vkdFile[(15++ = 16)] = optarg; }

Cheers,
--
Thomas E. Spanjaard

#5 Updated by corecode about 7 years ago

heh, no :)

% cat<<EOF>d.c
main(){int i=0;printf("%d\n", i++);}
EOF
% cc d.c
% ./a.out
0
%

cheers
simon

#6 Updated by c.turner about 7 years ago

On Sun, 28 Jan 2007 18:08:58 +0100
"Simon 'corecode' Schubert" <> wrote:

yeah.. should have written a test.. *doh*. the C book I referred to (in
*only* 24 hours!!) says '++' is higher in the ops than '=' .. blah. Too
bad the 'standard' costs $$

I think the worry was that = happens after ++ and the [] deref..

anyhoo, your right should be safe (and time to get another book)

$ cat assign-incr.c
main () {
int x[2]= {-1,-1};
int i = 0;
x[i++] = i;
printf("%d, %d\n", x[0], x[1]);
return i;
}
$ cc assign-incr.c
$ ./a.out
0, -1
$ echo $?
1
$

#7 Updated by jontro about 7 years ago

++i returns the new value of i and i++ returns i before being
incremented. See it as a function in that matter.

//Jonas

#8 Updated by swildner about 7 years ago

I've cleaned it up a bit and added some words to the man page. Please
check out http://leaf.dragonflybsd.org/~swildner/multivkd.diff

Can you cvsup and try again? This was due to a bug in get_empty_drive().
When I try your example now, I get:

# cat vinum.conf
drive d1 device /dev/vkd1a
drive d2 device /dev/vkd2a
drive d3 device /dev/vkd3a
drive d4 device /dev/vkd4a

# vinum create -f vinum.conf
Can't open /dev/vinum/controld: No such file or directory
1: drive d1 device /dev/vkd1a
** 1 Can't initialize drive d1: Operation not supported by device
2: drive d2 device /dev/vkd2a
** 2 Can't initialize drive d2: Operation not supported by device
3: drive d3 device /dev/vkd3a
** 3 Can't initialize drive d3: Operation not supported by device
4: drive d4 device /dev/vkd4a
** 4 Can't initialize drive d4: Operation not supported by device
rm: /dev/vinum: Directory not empty
Can't create /dev/vinum/controld: File exists
0 drives:
0 volumes:
0 plexes:
0 subdisks:

Well, regardless of the vinum issue, I think what we already have is
useful enough to be committed.

The only thing I'm wondering about is if we should rename the '-r'
option to the more general '-D' (analogous to -I). Any opinions on this?

I'll commit the patch next weekend if no one objects.

Sascha

#9 Updated by swildner about 7 years ago

Heh, some of those messages were due to me accidentally running two
vkernels using the same disk image.

Here's the correct output:

# vinum create -f vinum.conf
1: drive d1 device /dev/vkd1a
** 1 Can't initialize drive d1: Operation not supported by device
2: drive d2 device /dev/vkd2a
** 2 Can't initialize drive d2: Operation not supported by device
3: drive d3 device /dev/vkd3a
** 3 Can't initialize drive d3: Operation not supported by device
4: drive d4 device /dev/vkd4a
** 4 Can't initialize drive d4: Operation not supported by device
Can't open /dev/vinum/controld: No such file or directory
0 drives:
0 volumes:
0 plexes:
0 subdisks:

Sascha

#10 Updated by c.turner about 7 years ago

Sascha Wildner wrote:
> I've cleaned it up a bit and added some words to the man page. Please
> check out http://leaf.dragonflybsd.org/~swildner/multivkd.diff

Nifty - I was thinking of pestering the lists to check on it -
because I just *know* someone had to be using it, especially with
all this new-fangled-filesystem-talk :)

> Can you cvsup and try again? This was due to a bug in get_empty_drive().

will do - although my -CURRENT machines are unavailable at the moment,
so I'll probably manually apply to 1.8 release until a week or so from
now.

Another thing to be noted was that vinum assumes it's using a kernel
module generally, and the vinum docs warn against a static build,
and I had to statically built into the VKERNEL since module support for
vkernels isn't? there yet?...

> Well, regardless of the vinum issue, I think what we already have is
> useful enough to be committed.

thanks & IMHO agree

> The only thing I'm wondering about is if we should rename the '-r'
> option to the more general '-D' (analogous to -I). Any opinions on this?

personally I wasn't sure what to use, as '-r' didn't seem to fit,
so -D works and makes more sense (to me) as well.

I'm probably going to look at various command line / config file kinds
of things for the vkernel 'CLI' if you call it that in about a month or
so - right now I'm working on getting my systems installed in a very
repeatable way for some performance & stress tests I want to do shortly.

> I'll commit the patch next weekend if no one objects.

woohoo! thanks!

- Chris

#11 Updated by swildner about 7 years ago

Check out the -U option.

Sascha

#12 Updated by swildner about 7 years ago

Make sure you also take rev. 1.21 of sys/dev/raid/vinum/vinumio.c (see
commits@). Unfortunately I can't test much myself at the moment.

Sascha

#13 Updated by swildner about 7 years ago

I've committed it, thanks!

In the I didn't change -r to -D in the end because we have already
released with vkernel support and it's really not so important anyways.

Sascha

#14 Updated by c.turner about 7 years ago

think I responded to the direct copy.. don't want to seem rude, so
thanks again!

Also available in: Atom PDF