DragonFlyBSD

:Disclaimer: MESSY!
:
:Took a quick attempt at getting the VKERNEL to shutdown cleanly against
:1.8,
:and it seems to work just fine as suggested with mailbox signals, etc.
:
:Attached is my 'cumulative' 1.8 vkernel patch against 1.8_Release
:/usr/src/sys/platform/vkernel including the previous multi-vk changes.
:...
:status:
: - OK for simple shutdown case with init already fork()'ed.

Looks good.

: - VK is too fast for me to catch the 'no init yet' shutdown case..

Heh.

: - need to merge against Preview, as things have changed there,
: and re-submit. patch here includes previous multivkd work.

Yup, I was just going to say that... multi-vk is already in HEAD.

:changes in:
: - platform/vkernel/include/globaldata.h : maibox slot
: - platform/vkernel/include/md_var.h : vk_shutdown prototype
: - platform/vkernel/platform/kqueue.c : mailbox hook
: - platform/vkernel/platform/init.c : vk_shutdown function
:
:questions/notes:
: - I didn't think I really needed a separate sigaction in
: kqueue_setup..
: - should shutdown mailbox handler dispatch happen higher up
: (e.g. in platform/vkernel/i386/trap.c)
: - should this actually follow
: 'vke_intr(void *xsc, struct intrframe *frame __unused)' conventions?
: - we're not really checking for FD related IO kevents() ..
: - this implies a more 'full device' kind of thing , with a struct,
: etc.

The sigaction initialization for SIGTERM doesn't really belong
    in init_kqueue().  Put it in platform/vkernel/i386/exception.c
    instead.

Now the question is: should it be a signal mailbox or a real signal?

That's a good question.  Well, we have an issue with the small size
    of the kernel thread stack, so I think it should be a mailbox.

Now... when to check for and call the function!  There are two places
    where it needs to be checked:

(1) in go_user().
    (2) in cpu_idle().

Check for the mailbox and call a function specific to the shutdown
    (which you can also put in platform/vkernel/i386/exception.c).  Don't
    integrate it into signalmailbox().

: - should the other init(8) signals for e.g. single user, etc. be
: implemented? Currently, these are not implement as shutdown is
: ~90% of cases, and wanted to verify I'm taking the proper direction.

single-user mode on the master host will send a HUP or a TERM
    (I forget which) to all processes, including the vkernel.  They
    should probably be treated as a shutdown by the vkernel.

: - mailbox signals aren't really documented in the manual at the
: moment..

I'll do a quick fix to the sigaction manual page.  hmm.  I thought
    I had documented it.

:Comments / Questions / feedback welcome.
:
:Thanks!
:
: - Chris

realise I hadn't responsed .. thanks for feeback. will try to cleanup
soonish & resubmit.

how's HEAD stabilizing, or is Preview a better bet for patch testing?

Thanks,

- Chris

:realise I hadn't responsed .. thanks for feeback. will try to cleanup
:soonish & resubmit.
:
:how's HEAD stabilizing, or is Preview a better bet for patch testing?
:
:Thanks,
:
:- Chris

Preview.  HEADs gonna be a moving target for a while.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

Patch not committed. Feature added through a different approach though, see
http://leaf.dragonflybsd.org/mailarchive/commits/2007-05/msg00228.html .

This patch should't be committed - it's messy and not the way Matt
suggested I clean it up, but the commit referenced is absolutely not
the same thing so this shouldn't be marked resolved - unless people
don't want the feature anymore, in which case I'll continue to maintain
the patch for as long as I'm interested in it.

The functionality implemented for this issue (#647), the host system
is able to signal the guest VKERNEL to shut down cleanly (e.g. when
the host does a halt and forgets to shut down the guest VK's),

With the committed code above, the VK is able to shut itself
down cleanly, but the host continues to have no way of doing so -
Signals from outside will kill the VK and leave the VKD's in an
inconsistant state, possibly resulting in a loss of data within the VK
system

I responded to Matt's commit because I had a second patch in mind that
did the same thing (vkernel self-initiated shutdown/reboot) - was
probably going to tackle that later... as I see it, the two
compliment each other..

In any case,
hoping to clean up / resubmit this weekend, more than likely with a
second signal handler to take advantage of Matt's new reboot code.

Thanks,

- Chris

On Wed, May 30, 2007 8:47 pm, Chris Turner wrote:

I think it would be useful - it should be possible to start AND stop a
vkernel without having to actually be in the virtual environment. I'd
probably be more antsy about it if I had a spare 1.9 machine to experiment
on right now.

I guess I was too eager to close issues on the tracker, sorry.

Please do! Keep up the good work.

Cheers,

-Max

:
:In any case,
:hoping to clean up / resubmit this weekend, more than likely with a
:second signal handler to take advantage of Matt's new reboot code.
:
:Thanks,
:
:- Chris

Definitely!  Having a properly working shutdown signal would be good.
    In fact, it might be easiest to just set it up as a mailbox signal and
    monitor it in the clock interrupt, then create a kernel thread to initiate
    the actual shutdown when the signal is detected.  Or something like that.

--

I noticed another thing, which is when you run the vkernel from
    a shell script, and you interrupt it with ^\ + ^C (that sequence
    will enter ddb, then kill the vkernel), it currently causes the vkernel
    to do its shutdown/reboot thing, but also kills the shell script and
    disconnects the terminal so you wind up getting a shell prompt with the
    vkernel still running in the background.  Its very odd and annoying
    behavior.

I think there is some sort of termios / tty session magic we have
    to do to make the tty interrupt signals only go to the vkernel,
    but I'm not sure what it is.

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

OK.. Updated / cleaned this against -HEAD (as of this morning) and
things appear to be working.

(vk still too fast to catch the 'no init yet' case)

As suggested:

- moved shutdown hook and sigaction setup to
    sys/platform/vkernel/i386/exception.c.

- changed mailbox check to :

- /sys/platform/vkernel/i386/trap.c,v : go_user()
    - /sys/platform/vkernel/i386/cpu_regs.c,v : cpu_idle()

- kept mailbox flag in platform/vkernel/include/md_var.h

But:

- I'm still not 100% on the how the SIGIO mailbox interlock
    works, so the mailbox check hooks might be in the wrong /
    non-optimal places within go_user() / cpu_idle().
  - There isn't really a clean way of getting at the shutdown function
    from oustide 'sys/kern/kern_shutdown.c'. This results in compiler
    warnings, but the linker sorts things out.
  - Updated vkernel manual page also attached with patch - hopefully
    initial meaning was kept.
  - Didn't implement any other signals yet as am still getting approach
    sorted out.

p.s:

- now that there are two mailbox signals, perhaps signalmailbox()
     should and the related mdvar should be renamed e.g. iomailbox() ?
     didn't do this as not really directly related to the patch..
   - .. and mailbox signals still a bit fuzzy in the manual :)

Thanks

- Chris

:OK.. Updated / cleaned this against -HEAD (as of this morning) and
:things appear to be working.
:
:(vk still too fast to catch the 'no init yet' case)

This looks quite nice.  I just realized though that it isn't safe
    to run reboot() (or probably ksignal either) from the idle halt
    code, so scratch that part.

We have to find a good place to put this.  I think what we may have
    to do is create a kernel thread whos only job is to shut the kernel
    down, and then check the mailbox from inside signalmailbox() and have
    it wake the special kernel thread up.

Would you like to have a go at that or would you like me to do it?

-Matt

thanks. ok.

If the special thread doesn't need to do any other cleanup that I'm
likely unaware of, I can give it another shot this weekend.

Not yet knowing the overall code paths unfortunately means I'm
kind of coding this blind .. but I suppose the point is to learn as I go.

I assume the reason for the thread is that the way things are currently
called, the VK might not be in a consistent internal state when the
signal is delivered to init and/or init is rescheduled to run?

to clarify:

- keep signal handler and shutdown function in the exception setup
 - create kthread somewhere ( main() ? platform init? )
 - remove checks from both idle loop / task switching areas
 - add check / wakeup logic to signalmailbox()

Thanks,

- Chris

:If the special thread doesn't need to do any other cleanup that I'm
:likely unaware of, I can give it another shot this weekend.
:
:Not yet knowing the overall code paths unfortunately means I'm
:kind of coding this blind .. but I suppose the point is to learn as I go.
:
:I assume the reason for the thread is that the way things are currently
:called, the VK might not be in a consistent internal state when the
:signal is delivered to init and/or init is rescheduled to run?

Right.  The idle loop isn't allowed to block, and the shutdown
    sequence involves a lot of blocking.  The go-user code is in a
    sane state and it would be ok to block there, but we wouldn't
    get consistent checks.

:to clarify:
:
: - keep signal handler and shutdown function in the exception setup
: - create kthread somewhere ( main() ? platform init? )
: - remove checks from both idle loop / task switching areas
: - add check / wakeup logic to signalmailbox()
:
:Thanks,
:
:- Chris

I think the platform init code is too early.   The memory subsystem
    isn't really initialized until it gets into the middle mi_startup().

What I recommend is that you create a SYSINIT in
    platform/vkernel/i38/exception.c with a priority of SI_BOOT2_MACHDEP
    to create the thread.  This looks kinda messy, but it should work.

static void
    sigshutdown_daemon(void)
{
    while (mdcpu->gd_shutdown == 0)
        tsleep(&mdcpu->gd_shutdown, 0, "sswait", 0);
    }
    mdcpu->gd_shutdown = 0;
    kprintf("Caught SIGTERM from host system. Shutting down...\n");
    if (initproc != NULL) {
        ksignal(initproc, SIGUSR2);
    } else {
        reboot(RB_POWEROFF);
    }
    }

static struct thread *sigshutdown_thread;

static struct kproc_desc sigshut_kp = {
    "sigshutdown", sigshutdown_daemon, &sigshutdown_thread
    };

SYSINIT(sigshutdown, SI_BOOT2_MACHDEP, SI_ORDER_ANY,
        kproc_start, &sigshut_kp);

Then put a check in signalmbox which wakes up the thread:

/*
 * we only need to wake up our shutdown thread once.  Keep it non-zero
 * so the shutdown thread can detect it.
     */
    if (mdcpu->gd_shutdown > 0) {
    mdcpu->gd_shutdown = 1;
    wakeup(&mdcpu>gd_shutdown);
    }

-Matt
                    Matthew Dillon 
                    <dillon@backplane.com>

Okay. All set (+= roff updates per Sascha).

You'd mostly done all the work.. but now I understand the sysinit
things a bit more so that's good for something - and thanks! Would have
taken me all day to track things down, although my ctags-foo is improving :)

One thing that I did change (aside from minor <75 column reformatting, etc)
was that in the SYSINIT with the SI_BOOT2_MACHDEP, I'd get a panic before
init.. (see below)

I started to trace through this this but thought 'why not just try setting
this up later on in the sequence' which is why this is now set to
SI_BOOT2_PROC0, which is the lowest-numbered item in the sysinit_sub_id
enum that worked for me (TM).

Also, I'm wondering why you suggested resetting the mailbox flag to -1
when it is already nonzero.. kept this as is, but just for curiosity..

I'll knock out the pidfile thing in a few hours time.

Thanks,

- Chris

below:

real memory = 33554432 (32768K bytes)
avail memory = 30138368 (29432K bytes)
initclocks
panic: zone: invalid zone
Trace beginning at frame 0xbfbff858
(null)(8211300,828cf40,822e45f,bfbff888,0) at 0x80b5c20
(null)() at 0x80b5c20
Debugger("panic")
Stopped at 0x81f9615: movb $0,0x82bb898
db> trace
(null)(8211189,828cf40,822e45f,bfbff888,0) at 0x81f9615
(null)() at 0x80b5c35
db>

Chris Turner wrote:

updated -u format, per Sacha. Good eyes! (or bad ones on my part :)

Thanks,

- Chris

:
:updated u format, per Sacha. Good eyes! (or bad ones on my part :)
:
:Thanks,
:
: Chris

Looks good.  I did a quick test and it worked great.

I am committing it now!

-Matt