Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Module problem on amd64

On Sun, 30 Nov 2008, Andrew Doran wrote:

On Fri, Nov 28, 2008 at 04:54:46PM -0800, Paul Goyette wrote:

Well, even if noone else thinks it is useful, I went and did the deed!

Cool, thanks! I think there is MD code that plays with 'mountroot'. See:

TEST1 and TEST2 boot successfully to single user, and I'm able to do a
'ls' command from the single-user shell prompt.  However, both of these
panic due to the "end <= VM_MAX_KERNEL_ADDRESS" thing mentioned in an
earlier thread.

I will fix that soon.

+               /*
+                * If user specified a file system type, use only
+                * that type
+                */
+               if (strcmp(rootfstype, "?") && strcmp(rootfstype, v->vfs_name))
+                       continue;

KNF says 'strcmp(a, b) == 0'

Well, hopefully you meant 'strcmp(a,b) != 0' ??  I have fixed this.

+       static char my_vfs_name[NAME_MAX + 1];

Should be MNAMELEN from mount.h, I think.

OK, that's easy to fix.

+                               /*
+                                * Copy the vfs_name since *vops might be
+                                * deallocated?
+                                */
+                               strncpy(my_vfs_name, vops->vfs_name,
+                                       sizeof(my_vfs_name));
+                               rootfstype = (const char *)my_vfs_name;

Can you make 'rootfstype' in ioconf.c (or wherever it is emitted) into an
array sized MNAMELEN and not a pointer? That way, we can get rid of
'mountroot' completely and always key on the name. So the code in
vfs_mountroot() would look like:

It's emitted in config's mkswap.c, and defined in sys/systm.h.

I've already completely removed the mountroot variable; everything already keys only on the name. The only benefit of allocating the new rootfstype as a fixed array is to avoid having the local 'my_vfs_name' above.

But, if I define it as an array in swapnetbsd.c, it would also need to be defined that way is sys/sys/systm.h, and then either systm.h would need to #include mount.h, or all of its users would need to do that.

So, which is more desirable?

        1. #include <mount.h> in systm.h?
        2. #include <mount.h> in all users of systm.h?
        3. Have my_vfs_name statically allocated in kern_subr.c, retain
           rootfstype as a simple pointer, and make rootfstype point to
           it if needed?

        if (strcmp(rootfstype, "?") != 0 &&
            (v = vfs_getopsbyname(rootfstype)) != NULL) {
                if (v->vfs_mountroot != NULL) {
                        error = (*(v->vfs_mountroot))();
                        goto done;
                } else {

I have just noticed a bug in the existing code. In setroot():

If vops->vfs_mountroot == NULL, it should do vfs_delref(vops) before
continuing on.

I've added the additional call to vfs_delref() as follows:

                        vops = vfs_getopsbyname(buf);
                        if (vops == NULL || vops->vfs_mountroot == NULL) {
                                printf("use one of: generic");
                                for (vops = LIST_FIRST(&vfs_list);
                                     vops != NULL;
                                     vops = LIST_NEXT(vops, vfs_list)) {
                                        if (vops->vfs_mountroot != NULL)
                                                printf(" %s", vops->vfs_name);
                                if (vops != NULL)
#if defined(DDB)
                                printf(" ddb");
                                printf(" halt reboot\n");
                        } else {

I'm going to add another test kernel to force me into the RB_ASKNAME parts of setroot() just to verify that that code path works.

|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | |

Home | Main Index | Thread Index | Old Index