Date: Wed, 24 Oct 2007 08:43:03 GMT From: Marko Zec <zec@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 127995 for review Message-ID: <200710240843.l9O8h3U5096034@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=127995 Change 127995 by zec@zec_tpx32 on 2007/10/24 08:43:01 Rewrite a function for constructing full path name to a vimage, which was previously not only yielding bogus results, but was recursively invoking itself, which was not a wise approach given the limited size of the kernel stack. Rewrite vimage_get_next() function so that it doesn't traverse through the entire list of vimages, but walks the hierarchy by looking at child/parent/sibling relations. Change the convention for addressing parent vnet when returning an ifnet back to parent: instead of "-" now ".." is used. Fix spelling: vi_sibilings -> vi_sibling Do not expose vimage ID to userspace any more, since only symbolic vimage name is relevant for addressing purposes. Affected files ... .. //depot/projects/vimage/src/sys/kern/kern_vimage.c#51 edit .. //depot/projects/vimage/src/sys/sys/vimage.h#48 edit Differences ... ==== //depot/projects/vimage/src/sys/kern/kern_vimage.c#51 (text+ko) ==== @@ -285,7 +285,7 @@ * passed as ifp. The interface will be renamed to vi_req->vi_parent_name * if vi_req->vi_parent_name is not an empty string (uff ugly ugly)... * Similary, the target vnet can be specified either by vnet argument or - * by name. If vnet name equals to "-" or vi_req is set to NULL the + * by name. If vnet name equals to ".." or vi_req is set to NULL the * interface is moved to the parent vnet. */ int @@ -298,7 +298,7 @@ struct vnet *new_vnet = NULL; u_char eaddr[6]; - if (vi_req == NULL || strcmp(vi_req->vi_name, "-") == 0) { + if (vi_req == NULL || strcmp(vi_req->vi_name, "..") == 0) { if (IS_DEFAULT_VIMAGE(vip)) return (ENXIO); new_vnet = vip->vi_parent->v_net; @@ -465,7 +465,7 @@ namelen = strlen(name); if (namelen == 0) return(NULL); - LIST_FOREACH(vip, &top->vi_child_head, vi_sibilings) + LIST_FOREACH(vip, &top->vi_child_head, vi_sibling) if (strncmp(name, vip->vi_name, namelen) == 0) { if (next_name != NULL) return(vimage_by_name(vip, next_name)); @@ -476,40 +476,58 @@ } -static int +static void vimage_relative_name(struct vimage *top, struct vimage *where, char *buffer, int bufflen) { + int used = 1; + if (where == top) { sprintf(buffer, "."); - return(1); - } + return; + } else + *buffer = 0; - if (where->vi_parent != top) { - int len; + do { + int namelen = strlen(where->vi_name); - len = vimage_relative_name(top, where->vi_parent, - buffer, bufflen); - bufflen -= (len + 1); - buffer += len; - sprintf(buffer++, "."); - } + if (namelen + used + 1 >= bufflen) + panic("buffer overflow"); - sprintf(buffer, "%s", where->vi_name); - return(strlen(where->vi_name)); + if (used > 1) { + bcopy(buffer, &buffer[namelen + 1], used); + buffer[namelen] = '.'; + used++; + } else + bcopy(buffer, &buffer[namelen], used); + bcopy(where->vi_name, buffer, namelen); + used += namelen; + where = where->vi_parent; + } while (where != top); } static struct vimage * vimage_get_next(struct vimage *top, struct vimage *where) { + struct vimage *next; + + /* Try to go deeper in the hierarchy */ + next = LIST_FIRST(&where->vi_child_head); + if (next != NULL) + return(next); + do { - where = LIST_NEXT(where, vi_le); - if (where == NULL) - where = LIST_FIRST(&vimage_head); - if (vi_child_of(top, where)) - return(where); - } while (where != top); + /* Try to find next sibling */ + next = LIST_NEXT(where, vi_sibling); + if (next != NULL) + return(next); + + /* Nothing left on this level, go one level up */ + where = where->vi_parent; + } while (where != top->vi_parent); + + /* Nothing left to be visited, we are done */ return(NULL); } @@ -546,7 +564,6 @@ switch (cmd) { case SIOCGPVIMAGE: - vi_req->vi_id = vip_r->vi_id; vimage_relative_name(vip, vip_r, vi_req->vi_name, sizeof (vi_req->vi_name)); bcopy(&vip_r->v_procg->_averunnable, &vi_req->averunnable, @@ -675,7 +692,7 @@ vip->vi_parent = parent; /* XXX locking */ if (parent != NULL) - LIST_INSERT_HEAD(&parent->vi_child_head, vip, vi_sibilings); + LIST_INSERT_HEAD(&parent->vi_child_head, vip, vi_sibling); else if (!LIST_EMPTY(&vimage_head)) panic("there can be only one default vimage!"); LIST_INSERT_HEAD(&vimage_head, vip, vi_le); @@ -755,7 +772,7 @@ /* Point with no return - cleanup MUST succeed! */ /* XXX locking */ LIST_REMOVE(vip, vi_le); - LIST_REMOVE(vip, vi_sibilings); + LIST_REMOVE(vip, vi_sibling); /* XXX locking */ LIST_REMOVE(vprocg, vprocg_le); ==== //depot/projects/vimage/src/sys/sys/vimage.h#48 (text+ko) ==== @@ -379,7 +379,7 @@ struct vimage { LIST_ENTRY(vimage) vi_le; /* all vimage list */ - LIST_ENTRY(vimage) vi_sibilings; /* vimages with same parent */ + LIST_ENTRY(vimage) vi_sibling; /* vimages with same parent */ LIST_HEAD(, vimage) vi_child_head; /* direct offspring list */ struct vimage *vi_parent; /* ptr to parent vimage */ u_int vi_id; /* ID num */ @@ -461,7 +461,6 @@ u_int vi_cpu_weight; /* Prop. share scheduling priority */ int vi_intr_limit; /* Limit on CPU usage in intr ctx */ int vi_maxsockets; - u_short vi_id; /* IDnum - but do we need it at all? */ u_short vi_proc_limit; /* max. number of processes */ u_short vi_proc_count; /* current number of processes */ u_short vi_child_limit; /* max. number of child vnets */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200710240843.l9O8h3U5096034>