-
Notifications
You must be signed in to change notification settings - Fork 63
Nfs volume #252
Nfs volume #252
Conversation
Signed-off-by: Peng Tao <bergwolf@gmail.com>
build/make-initrd.sh
Outdated
| cp busybox /tmp/hyperstart-rootfs | ||
| cp iptables /tmp/hyperstart-rootfs | ||
| cp libm.so.6 /tmp/hyperstart-rootfs/lib64/ | ||
| cp mount.nfs /tmp/hyperstart-rootfs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please move it to /tmp/hyperstart-rootfs/sbin/?
build/make-initrd.sh
Outdated
| do | ||
| mkdir -p /tmp/hyperstart-rootfs/`dirname ${bin}` | ||
| ln -sf /mount.nfs /tmp/hyperstart-rootfs/${bin} | ||
| done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this block of code is unnecessary.
src/container.c
Outdated
|
|
||
| if (!strncmp(vol->fstype, "xfs", strlen("xfs"))) | ||
| options = "nouuid"; | ||
| if (!strncmp(vol->fstype, "nfs", strlen("nfs"))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you use strcmp()?
strncmp() is bug here, hyperstart is full of such bugs....
Use external mount.nfs binary (nfs-utils-1-3-4, 188354e57) to avoid adding too much complexity (viz, rpc mnt protocol support). Signed-off-by: Peng Tao <bergwolf@gmail.com>
|
@laijs updated. |
|
LGTM, and where is the |
|
mount.nfs was built from nfs-utils(http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=summary) tag nfs-utils-1-3-4 |
| if (hyper_mount_nfs(vol->device, path) < 0) | ||
| return -1; | ||
| /* nfs export has implicitly included _data part of the volume */ | ||
| sprintf(volume, "/%s/", path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this affect container_populate_volume?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update: yes it does. populate will be disabled with nfs volumes, mostly because if the client creates a _data directory, it will be visible to the server and violates the idea of importing volumes from server because _data directory is not really a volume mountpoint of the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine, this pr LGTM
|
retest this please @hykins |
|
LGTM |
This implements the same as #248 but works with latest master instead.