* [PATCH 1/2] kvm tools: Add support for 9p2000.u
@ 2011-08-01 14:08 Sasha Levin
2011-08-01 14:08 ` [PATCH 2/2] kvm tools: Add '--rootfs' and '--binsh' Sasha Levin
2011-08-01 15:12 ` [PATCH 1/2] kvm tools: Add support for 9p2000.u Aneesh Kumar K.V
0 siblings, 2 replies; 5+ messages in thread
From: Sasha Levin @ 2011-08-01 14:08 UTC (permalink / raw)
To: penberg; +Cc: kvm, aneesh.kumar, mingo, asias.hejun, gorcunov, Sasha Levin
This patch adds support for the UNIX extensions to 9p2000.
Supporting thses extensions allow us to transperantly mount UNIX directories
without missing features such as symlinks.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/include/kvm/virtio-9p.h | 4 +-
tools/kvm/virtio/9p-pdu.c | 6 ++-
tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------
3 files changed, 69 insertions(+), 16 deletions(-)
diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
index 8584f49..0e55e5c 100644
--- a/tools/kvm/include/kvm/virtio-9p.h
+++ b/tools/kvm/include/kvm/virtio-9p.h
@@ -11,8 +11,8 @@
#define VIRTQUEUE_NUM 128
#define VIRTIO_P9_DEFAULT_TAG "kvm_9p"
#define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16))
-#define VIRTIO_P9_MAX_FID 128
-#define VIRTIO_P9_VERSION "9P2000"
+#define VIRTIO_P9_MAX_FID 256
+#define VIRTIO_P9_VERSION "9P2000.u"
#define MAX_TAG_LEN 32
struct p9_msg {
diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c
index 0c454db..8ed249f 100644
--- a/tools/kvm/virtio/9p-pdu.c
+++ b/tools/kvm/virtio/9p-pdu.c
@@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap)
case 'S':
{
struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
- retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss",
+ retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd",
stbuf->size, stbuf->type,
stbuf->dev, &stbuf->qid,
stbuf->mode, stbuf->atime,
stbuf->mtime, stbuf->length,
stbuf->name, stbuf->uid,
- stbuf->gid, stbuf->muid);
+ stbuf->gid, stbuf->muid,
+ stbuf->extension, stbuf->n_uid,
+ stbuf->n_gid, stbuf->n_muid);
}
break;
default:
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 3b5555c..a6eafe0 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev,
err_str = strerror(err);
pdu->write_offset = VIRTIO_P9_HDR_LEN;
- virtio_p9_pdu_writef(pdu, "s", err_str);
+ virtio_p9_pdu_writef(pdu, "sd", err_str, -err);
*outlen = pdu->write_offset;
pdu->read_offset = sizeof(u32) + sizeof(u8);
@@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev,
struct p9_qid qid;
struct p9_fid *new_fid;
-
virtio_p9_pdu_readf(pdu, "db", &fid, &mode);
new_fid = &p9dev->fids[fid];
@@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev,
u8 mode;
u32 perm;
char *name;
+ char *ext = NULL;
u32 fid_val;
struct stat st;
struct p9_qid qid;
struct p9_fid *fid;
char full_path[PATH_MAX];
- virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode);
+ virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode, &ext);
fid = &p9dev->fids[fid_val];
sprintf(full_path, "%s/%s", fid->abs_path, name);
@@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev,
close_fid(p9dev, fid_val);
fid->dir = dir;
fid->is_dir = 1;
+ } else if (perm & P9_DMSYMLINK) {
+ int r;
+
+ r = symlink(ext, full_path);
+ if (r < 0)
+ goto err_out;
+ fd = open(full_path, omode2uflags(mode));
+ if (fd < 0)
+ goto err_out;
+ close_fid(p9dev, fid_val);
+ fid->fd = fd;
+ } else if (perm & P9_DMLINK) {
+ int r;
+ int ext_fid = atoi(ext);
+
+ r = link(p9dev->fids[ext_fid].abs_path, full_path);
+ if (r < 0)
+ goto err_out;
+
+ fd = open(full_path, omode2uflags(mode));
+ if (fd < 0)
+ goto err_out;
+ close_fid(p9dev, fid_val);
+ fid->fd = fd;
} else {
fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777);
if (fd < 0)
@@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
u32 newfid_val;
struct p9_qid wqid;
struct p9_fid *new_fid;
-
+ int ret;
virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
new_fid = &p9dev->fids[newfid_val];
@@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
/* Format the new path we're 'walk'ing into */
sprintf(tmp, "%s/%.*s",
fid->path, (int)strlen(str), str);
- if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
+ if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) {
+ ret = ENOENT;
goto err_out;
+ }
st2qid(&st, &wqid);
new_fid->is_dir = S_ISDIR(st.st_mode);
@@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
virtio_p9_set_reply_header(pdu, *outlen);
return;
err_out:
- virtio_p9_error_reply(p9dev, pdu, errno, outlen);
+ virtio_p9_error_reply(p9dev, pdu, ret, outlen);
return;
}
@@ -381,6 +407,12 @@ err_out:
return;
}
+static void virtio_p9_free_stat(struct p9_wstat *wstat)
+{
+ free(wstat->extension);
+ free(wstat->name);
+}
+
static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
struct stat *st, struct p9_wstat *wstat)
{
@@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
wstat->length = 0;
wstat->mode |= P9_DMDIR;
}
+ if(S_ISLNK(st->st_mode)) {
+ char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0};
+
+ rel_to_abs(p9dev, name, full_path);
+
+ if (readlink(full_path, tmp, PATH_MAX) > 0)
+ wstat->extension = strdup(tmp);
+ wstat->mode |= P9_DMSYMLINK;
+ } else {
+ wstat->extension = NULL;
+ }
wstat->atime = st->st_atime;
wstat->mtime = st->st_mtime;
@@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
wstat->uid = NULL;
wstat->gid = NULL;
wstat->muid = NULL;
+ wstat->n_uid = wstat->n_gid = wstat->n_muid = 0;
- /* NOTE: size shouldn't include its own length */
- /* size[2] type[2] dev[4] qid[13] */
- /* mode[4] atime[4] mtime[4] length[8]*/
- /* name[s] uid[s] gid[s] muid[s] */
- wstat->size = 2+4+13+4+4+4+8+2+2+2+2;
+ /*
+ * NOTE: size shouldn't include its own length
+ * size[2] type[2] dev[4] qid[13]
+ * mode[4] atime[4] mtime[4] length[8]
+ * name[s] uid[s] gid[s] muid[s]
+ * ext[s] uid[4] gid[4] muid[4]
+ */
+ wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4;
if (wstat->name)
wstat->size += strlen(wstat->name);
+ if (wstat->extension)
+ wstat->size += strlen(wstat->extension);
}
static void virtio_p9_read(struct p9_dev *p9dev,
@@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev,
read = pdu->write_offset;
virtio_p9_pdu_writef(pdu, "S", &wstat);
rcount += pdu->write_offset - read;
+ virtio_p9_free_stat(&wstat);
cur = readdir(fid->dir);
}
@@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev,
virtio_p9_pdu_writef(pdu, "wS", 0, &wstat);
*outlen = pdu->write_offset;
+ virtio_p9_free_stat(&wstat);
virtio_p9_set_reply_header(pdu, *outlen);
return;
err_out:
--
1.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] kvm tools: Add '--rootfs' and '--binsh'
2011-08-01 14:08 [PATCH 1/2] kvm tools: Add support for 9p2000.u Sasha Levin
@ 2011-08-01 14:08 ` Sasha Levin
2011-08-01 15:12 ` [PATCH 1/2] kvm tools: Add support for 9p2000.u Aneesh Kumar K.V
1 sibling, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2011-08-01 14:08 UTC (permalink / raw)
To: penberg; +Cc: kvm, aneesh.kumar, mingo, asias.hejun, gorcunov, Sasha Levin
This patch adds 2 new flags:
--rootfs [path] - Specifies a path to use as rootfs. The path will be mounted
using virtio-9p and booted from.
Easiest way to test it is to mount the sample image we recommend with kvm tool
(http://wiki.qemu.org/download/linux-0.2.img.bz2) and mounting it somewhere.
*PLEASE DO NOT TRY BOOTING FROM YOUR REAL ROOT AKA '/', EVEN WITH READ ONLY
SET* - Lets make sure the code is bulletproof before we delete your rootfs :)
--binsh - This flag will boot into simple /bin/sh shell instead of going
through /sbin/init. This is usefull for more complex distribution rootfs
where we have to deal with /etc/fstab and such.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/builtin-run.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 2e04265..78680ec 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -76,11 +76,13 @@ static const char *guest_mac;
static const char *host_mac;
static const char *script;
static const char *guest_name;
+static const char *rootfs;
static bool single_step;
static bool readonly_image[MAX_DISK_IMAGES];
static bool vnc;
static bool sdl;
static bool balloon;
+static bool bin_sh;
extern bool ioport_debug;
extern int active_console;
extern int debug_iodelay;
@@ -156,6 +158,8 @@ static const struct option options[] = {
OPT_BOOLEAN('\0', "balloon", &balloon, "Enable virtio balloon"),
OPT_BOOLEAN('\0', "vnc", &vnc, "Enable VNC framebuffer"),
OPT_BOOLEAN('\0', "sdl", &sdl, "Enable SDL framebuffer"),
+ OPT_STRING('\0', "rootfs", &rootfs, "path", "Use path as rootfs"),
+ OPT_BOOLEAN('\0', "binsh", &bin_sh, "Boot rootfs to /bin/sh instead of /sbin/init"),
OPT_GROUP("Kernel options:"),
OPT_STRING('k', "kernel", &kernel_filename, "kernel",
@@ -615,7 +619,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
hi = NULL;
- if (!image_filename[0]) {
+ if (!rootfs && !image_filename[0]) {
hi = host_image(real_cmdline, sizeof(real_cmdline));
if (hi) {
image_filename[0] = hi;
@@ -627,6 +631,18 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
if (!strstr(real_cmdline, "root="))
strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
+ if (rootfs) {
+ char tmp[PATH_MAX];
+
+ if (realpath(rootfs, tmp) == 0 ||
+ virtio_9p__init(kvm, tmp, "/dev/root") < 0)
+ die("Unable to initialize virtio 9p");
+ strcat(real_cmdline, " root=/dev/root rootflags=rw,trans=virtio,version=9p2000.u rootfstype=9p");
+
+ if (bin_sh)
+ strcat(real_cmdline, " init=/bin/sh");
+ }
+
if (image_count) {
kvm->nr_disks = image_count;
kvm->disks = disk_image__open_all(image_filename, readonly_image, image_count);
--
1.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm tools: Add support for 9p2000.u
2011-08-01 14:08 [PATCH 1/2] kvm tools: Add support for 9p2000.u Sasha Levin
2011-08-01 14:08 ` [PATCH 2/2] kvm tools: Add '--rootfs' and '--binsh' Sasha Levin
@ 2011-08-01 15:12 ` Aneesh Kumar K.V
2011-08-01 15:26 ` Sasha Levin
1 sibling, 1 reply; 5+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-01 15:12 UTC (permalink / raw)
To: Sasha Levin, penberg; +Cc: kvm, mingo, asias.hejun, gorcunov, Sasha Levin
On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote:
> This patch adds support for the UNIX extensions to 9p2000.
>
> Supporting thses extensions allow us to transperantly mount UNIX directories
> without missing features such as symlinks.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> tools/kvm/include/kvm/virtio-9p.h | 4 +-
> tools/kvm/virtio/9p-pdu.c | 6 ++-
> tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------
> 3 files changed, 69 insertions(+), 16 deletions(-)
>
> diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
> index 8584f49..0e55e5c 100644
> --- a/tools/kvm/include/kvm/virtio-9p.h
> +++ b/tools/kvm/include/kvm/virtio-9p.h
> @@ -11,8 +11,8 @@
> #define VIRTQUEUE_NUM 128
> #define VIRTIO_P9_DEFAULT_TAG "kvm_9p"
> #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16))
> -#define VIRTIO_P9_MAX_FID 128
> -#define VIRTIO_P9_VERSION "9P2000"
> +#define VIRTIO_P9_MAX_FID 256
> +#define VIRTIO_P9_VERSION "9P2000.u"
> #define MAX_TAG_LEN 32
>
> struct p9_msg {
> diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c
> index 0c454db..8ed249f 100644
> --- a/tools/kvm/virtio/9p-pdu.c
> +++ b/tools/kvm/virtio/9p-pdu.c
> @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap)
> case 'S':
> {
> struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
> - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss",
> + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd",
> stbuf->size, stbuf->type,
> stbuf->dev, &stbuf->qid,
> stbuf->mode, stbuf->atime,
> stbuf->mtime, stbuf->length,
> stbuf->name, stbuf->uid,
> - stbuf->gid, stbuf->muid);
> + stbuf->gid, stbuf->muid,
> + stbuf->extension, stbuf->n_uid,
> + stbuf->n_gid, stbuf->n_muid);
> }
> break;
> default:
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index 3b5555c..a6eafe0 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev,
>
> err_str = strerror(err);
> pdu->write_offset = VIRTIO_P9_HDR_LEN;
> - virtio_p9_pdu_writef(pdu, "s", err_str);
> + virtio_p9_pdu_writef(pdu, "sd", err_str, -err);
Should that be -err ? I guess it should the errno value ?
> *outlen = pdu->write_offset;
>
> pdu->read_offset = sizeof(u32) + sizeof(u8);
> @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev,
> struct p9_qid qid;
> struct p9_fid *new_fid;
>
> -
> virtio_p9_pdu_readf(pdu, "db", &fid, &mode);
> new_fid = &p9dev->fids[fid];
>
> @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> u8 mode;
> u32 perm;
> char *name;
> + char *ext = NULL;
> u32 fid_val;
> struct stat st;
> struct p9_qid qid;
> struct p9_fid *fid;
> char full_path[PATH_MAX];
>
> - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode);
> + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode,
> &ext);
ext need to be freed. ? I guess we have other places where the memory
alloc did for readf(.., "s", ..) is not freed.
> fid = &p9dev->fids[fid_val];
>
> sprintf(full_path, "%s/%s", fid->abs_path, name);
> @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> close_fid(p9dev, fid_val);
> fid->dir = dir;
> fid->is_dir = 1;
> + } else if (perm & P9_DMSYMLINK) {
> + int r;
> +
> + r = symlink(ext, full_path);
> + if (r < 0)
> + goto err_out;
> + fd = open(full_path, omode2uflags(mode));
> + if (fd < 0)
> + goto err_out;
> + close_fid(p9dev, fid_val);
> + fid->fd = fd;
that updates the fid ? Is that needed ? Also above we are just updating
fid->fd where as fid->path still points to the old name. I guess
symlink and link don't result in open
> + } else if (perm & P9_DMLINK) {
> + int r;
> + int ext_fid = atoi(ext);
> +
> + r = link(p9dev->fids[ext_fid].abs_path, full_path);
> + if (r < 0)
> + goto err_out;
> +
> + fd = open(full_path, omode2uflags(mode));
> + if (fd < 0)
> + goto err_out;
> + close_fid(p9dev, fid_val);
> + fid->fd = fd;
> } else {
> fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777);
> if (fd < 0)
> @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> u32 newfid_val;
> struct p9_qid wqid;
> struct p9_fid *new_fid;
> -
> + int ret;
>
> virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
> new_fid = &p9dev->fids[newfid_val];
> @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> /* Format the new path we're 'walk'ing into */
> sprintf(tmp, "%s/%.*s",
> fid->path, (int)strlen(str), str);
> - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
> + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) {
> + ret = ENOENT;
> goto err_out;
> + }
Why ?. What error with lstat failed which we wanted to map to ENOENT ?
>
> st2qid(&st, &wqid);
> new_fid->is_dir = S_ISDIR(st.st_mode);
> @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> virtio_p9_set_reply_header(pdu, *outlen);
> return;
> err_out:
> - virtio_p9_error_reply(p9dev, pdu, errno, outlen);
> + virtio_p9_error_reply(p9dev, pdu, ret, outlen);
> return;
> }
>
> @@ -381,6 +407,12 @@ err_out:
> return;
> }
>
> +static void virtio_p9_free_stat(struct p9_wstat *wstat)
> +{
> + free(wstat->extension);
> + free(wstat->name);
> +}
> +
> static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> struct stat *st, struct p9_wstat *wstat)
> {
> @@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> wstat->length = 0;
> wstat->mode |= P9_DMDIR;
> }
> + if(S_ISLNK(st->st_mode)) {
> + char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0};
> +
> + rel_to_abs(p9dev, name, full_path);
> +
> + if (readlink(full_path, tmp, PATH_MAX) > 0)
> + wstat->extension = strdup(tmp);
> + wstat->mode |= P9_DMSYMLINK;
> + } else {
> + wstat->extension = NULL;
> + }
>
> wstat->atime = st->st_atime;
> wstat->mtime = st->st_mtime;
> @@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> wstat->uid = NULL;
> wstat->gid = NULL;
> wstat->muid = NULL;
> + wstat->n_uid = wstat->n_gid = wstat->n_muid = 0;
>
> - /* NOTE: size shouldn't include its own length */
> - /* size[2] type[2] dev[4] qid[13] */
> - /* mode[4] atime[4] mtime[4] length[8]*/
> - /* name[s] uid[s] gid[s] muid[s] */
> - wstat->size = 2+4+13+4+4+4+8+2+2+2+2;
> + /*
> + * NOTE: size shouldn't include its own length
> + * size[2] type[2] dev[4] qid[13]
> + * mode[4] atime[4] mtime[4] length[8]
> + * name[s] uid[s] gid[s] muid[s]
> + * ext[s] uid[4] gid[4] muid[4]
> + */
> + wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4;
> if (wstat->name)
> wstat->size += strlen(wstat->name);
> + if (wstat->extension)
> + wstat->size += strlen(wstat->extension);
> }
>
> static void virtio_p9_read(struct p9_dev *p9dev,
> @@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev,
> read = pdu->write_offset;
> virtio_p9_pdu_writef(pdu, "S", &wstat);
> rcount += pdu->write_offset - read;
> + virtio_p9_free_stat(&wstat);
>
> cur = readdir(fid->dir);
> }
> @@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev,
>
> virtio_p9_pdu_writef(pdu, "wS", 0, &wstat);
> *outlen = pdu->write_offset;
> + virtio_p9_free_stat(&wstat);
> virtio_p9_set_reply_header(pdu, *outlen);
> return;
> err_out:
> --
> 1.7.6
>
-aneesh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm tools: Add support for 9p2000.u
2011-08-01 15:12 ` [PATCH 1/2] kvm tools: Add support for 9p2000.u Aneesh Kumar K.V
@ 2011-08-01 15:26 ` Sasha Levin
2011-08-02 16:36 ` Aneesh Kumar K.V
0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2011-08-01 15:26 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: penberg, kvm, mingo, asias.hejun, gorcunov
On Mon, 2011-08-01 at 20:42 +0530, Aneesh Kumar K.V wrote:
> On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote:
> > This patch adds support for the UNIX extensions to 9p2000.
> >
> > Supporting thses extensions allow us to transperantly mount UNIX directories
> > without missing features such as symlinks.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> > tools/kvm/include/kvm/virtio-9p.h | 4 +-
> > tools/kvm/virtio/9p-pdu.c | 6 ++-
> > tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------
> > 3 files changed, 69 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
> > index 8584f49..0e55e5c 100644
> > --- a/tools/kvm/include/kvm/virtio-9p.h
> > +++ b/tools/kvm/include/kvm/virtio-9p.h
> > @@ -11,8 +11,8 @@
> > #define VIRTQUEUE_NUM 128
> > #define VIRTIO_P9_DEFAULT_TAG "kvm_9p"
> > #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16))
> > -#define VIRTIO_P9_MAX_FID 128
> > -#define VIRTIO_P9_VERSION "9P2000"
> > +#define VIRTIO_P9_MAX_FID 256
> > +#define VIRTIO_P9_VERSION "9P2000.u"
> > #define MAX_TAG_LEN 32
> >
> > struct p9_msg {
> > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c
> > index 0c454db..8ed249f 100644
> > --- a/tools/kvm/virtio/9p-pdu.c
> > +++ b/tools/kvm/virtio/9p-pdu.c
> > @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap)
> > case 'S':
> > {
> > struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
> > - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss",
> > + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd",
> > stbuf->size, stbuf->type,
> > stbuf->dev, &stbuf->qid,
> > stbuf->mode, stbuf->atime,
> > stbuf->mtime, stbuf->length,
> > stbuf->name, stbuf->uid,
> > - stbuf->gid, stbuf->muid);
> > + stbuf->gid, stbuf->muid,
> > + stbuf->extension, stbuf->n_uid,
> > + stbuf->n_gid, stbuf->n_muid);
> > }
> > break;
> > default:
> > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> > index 3b5555c..a6eafe0 100644
> > --- a/tools/kvm/virtio/9p.c
> > +++ b/tools/kvm/virtio/9p.c
> > @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev,
> >
> > err_str = strerror(err);
> > pdu->write_offset = VIRTIO_P9_HDR_LEN;
> > - virtio_p9_pdu_writef(pdu, "s", err_str);
> > + virtio_p9_pdu_writef(pdu, "sd", err_str, -err);
>
> Should that be -err ? I guess it should the errno value ?
Yup, but usually we get -errno back from functions and pass it to
virtio_p9_error_reply() while 9p expects it to be errno.
>
> > *outlen = pdu->write_offset;
> >
> > pdu->read_offset = sizeof(u32) + sizeof(u8);
> > @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev,
> > struct p9_qid qid;
> > struct p9_fid *new_fid;
> >
> > -
> > virtio_p9_pdu_readf(pdu, "db", &fid, &mode);
> > new_fid = &p9dev->fids[fid];
> >
> > @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> > u8 mode;
> > u32 perm;
> > char *name;
> > + char *ext = NULL;
> > u32 fid_val;
> > struct stat st;
> > struct p9_qid qid;
> > struct p9_fid *fid;
> > char full_path[PATH_MAX];
> >
> > - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode);
> > + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode,
> > &ext);
>
> ext need to be freed. ? I guess we have other places where the memory
> alloc did for readf(.., "s", ..) is not freed.
Oh heh, I took care of the part where we create wstat and write it, not
for the part where we read it :)
>
> > fid = &p9dev->fids[fid_val];
> >
> > sprintf(full_path, "%s/%s", fid->abs_path, name);
> > @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> > close_fid(p9dev, fid_val);
> > fid->dir = dir;
> > fid->is_dir = 1;
> > + } else if (perm & P9_DMSYMLINK) {
> > + int r;
> > +
> > + r = symlink(ext, full_path);
> > + if (r < 0)
> > + goto err_out;
> > + fd = open(full_path, omode2uflags(mode));
> > + if (fd < 0)
> > + goto err_out;
> > + close_fid(p9dev, fid_val);
> > + fid->fd = fd;
>
>
> that updates the fid ? Is that needed ? Also above we are just updating
> fid->fd where as fid->path still points to the old name. I guess
> symlink and link don't result in open
I'm really not sure if it's needed, the RFC isn't specific as to whether
we open 'special' files after creation or not, so I wanted to go with
opening them just in case (the kernel will clunk them anyway later, so
what the hell...).
Regarding the open in the symlink case, it's same deal really - I wasn't
sure whether create opens the file or not, and since theres no point in
opening a symlink I went ahead and opened the target of the symlink.
I didn't see a case where the kernel uses a linked/symlinked file
straight away, but as the RFC wasn't specific it was implemented anyway.
>
> > + } else if (perm & P9_DMLINK) {
> > + int r;
> > + int ext_fid = atoi(ext);
> > +
> > + r = link(p9dev->fids[ext_fid].abs_path, full_path);
> > + if (r < 0)
> > + goto err_out;
> > +
> > + fd = open(full_path, omode2uflags(mode));
> > + if (fd < 0)
> > + goto err_out;
> > + close_fid(p9dev, fid_val);
> > + fid->fd = fd;
> > } else {
> > fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777);
> > if (fd < 0)
> > @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > u32 newfid_val;
> > struct p9_qid wqid;
> > struct p9_fid *new_fid;
> > -
> > + int ret;
> >
> > virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
> > new_fid = &p9dev->fids[newfid_val];
> > @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > /* Format the new path we're 'walk'ing into */
> > sprintf(tmp, "%s/%.*s",
> > fid->path, (int)strlen(str), str);
> > - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
> > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) {
> > + ret = ENOENT;
> > goto err_out;
> > + }
>
> Why ?. What error with lstat failed which we wanted to map to ENOENT ?
stat of non-existent files.
>
> >
> > st2qid(&st, &wqid);
> > new_fid->is_dir = S_ISDIR(st.st_mode);
> > @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > virtio_p9_set_reply_header(pdu, *outlen);
> > return;
> > err_out:
> > - virtio_p9_error_reply(p9dev, pdu, errno, outlen);
> > + virtio_p9_error_reply(p9dev, pdu, ret, outlen);
> > return;
> > }
> >
> > @@ -381,6 +407,12 @@ err_out:
> > return;
> > }
> >
> > +static void virtio_p9_free_stat(struct p9_wstat *wstat)
> > +{
> > + free(wstat->extension);
> > + free(wstat->name);
> > +}
> > +
> > static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> > struct stat *st, struct p9_wstat *wstat)
> > {
> > @@ -393,6 +425,17 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> > wstat->length = 0;
> > wstat->mode |= P9_DMDIR;
> > }
> > + if(S_ISLNK(st->st_mode)) {
> > + char tmp[PATH_MAX] = {0}, full_path[PATH_MAX] = {0};
> > +
> > + rel_to_abs(p9dev, name, full_path);
> > +
> > + if (readlink(full_path, tmp, PATH_MAX) > 0)
> > + wstat->extension = strdup(tmp);
> > + wstat->mode |= P9_DMSYMLINK;
> > + } else {
> > + wstat->extension = NULL;
> > + }
> >
> > wstat->atime = st->st_atime;
> > wstat->mtime = st->st_mtime;
> > @@ -401,14 +444,20 @@ static void virtio_p9_fill_stat(struct p9_dev *p9dev, const char *name,
> > wstat->uid = NULL;
> > wstat->gid = NULL;
> > wstat->muid = NULL;
> > + wstat->n_uid = wstat->n_gid = wstat->n_muid = 0;
> >
> > - /* NOTE: size shouldn't include its own length */
> > - /* size[2] type[2] dev[4] qid[13] */
> > - /* mode[4] atime[4] mtime[4] length[8]*/
> > - /* name[s] uid[s] gid[s] muid[s] */
> > - wstat->size = 2+4+13+4+4+4+8+2+2+2+2;
> > + /*
> > + * NOTE: size shouldn't include its own length
> > + * size[2] type[2] dev[4] qid[13]
> > + * mode[4] atime[4] mtime[4] length[8]
> > + * name[s] uid[s] gid[s] muid[s]
> > + * ext[s] uid[4] gid[4] muid[4]
> > + */
> > + wstat->size = 2+4+13+4+4+4+8+2+2+2+2+2+4+4+4;
> > if (wstat->name)
> > wstat->size += strlen(wstat->name);
> > + if (wstat->extension)
> > + wstat->size += strlen(wstat->extension);
> > }
> >
> > static void virtio_p9_read(struct p9_dev *p9dev,
> > @@ -440,6 +489,7 @@ static void virtio_p9_read(struct p9_dev *p9dev,
> > read = pdu->write_offset;
> > virtio_p9_pdu_writef(pdu, "S", &wstat);
> > rcount += pdu->write_offset - read;
> > + virtio_p9_free_stat(&wstat);
> >
> > cur = readdir(fid->dir);
> > }
> > @@ -486,6 +536,7 @@ static void virtio_p9_stat(struct p9_dev *p9dev,
> >
> > virtio_p9_pdu_writef(pdu, "wS", 0, &wstat);
> > *outlen = pdu->write_offset;
> > + virtio_p9_free_stat(&wstat);
> > virtio_p9_set_reply_header(pdu, *outlen);
> > return;
> > err_out:
> > --
> > 1.7.6
> >
>
> -aneesh
--
Sasha.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] kvm tools: Add support for 9p2000.u
2011-08-01 15:26 ` Sasha Levin
@ 2011-08-02 16:36 ` Aneesh Kumar K.V
0 siblings, 0 replies; 5+ messages in thread
From: Aneesh Kumar K.V @ 2011-08-02 16:36 UTC (permalink / raw)
To: Sasha Levin; +Cc: penberg, kvm, mingo, asias.hejun, gorcunov
On Mon, 01 Aug 2011 18:26:23 +0300, Sasha Levin <levinsasha928@gmail.com> wrote:
> On Mon, 2011-08-01 at 20:42 +0530, Aneesh Kumar K.V wrote:
> > On Mon, 1 Aug 2011 17:08:22 +0300, Sasha Levin <levinsasha928@gmail.com> wrote:
> > > This patch adds support for the UNIX extensions to 9p2000.
> > >
> > > Supporting thses extensions allow us to transperantly mount UNIX directories
> > > without missing features such as symlinks.
> > >
> > > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > > ---
> > > tools/kvm/include/kvm/virtio-9p.h | 4 +-
> > > tools/kvm/virtio/9p-pdu.c | 6 ++-
> > > tools/kvm/virtio/9p.c | 75 +++++++++++++++++++++++++++++++------
> > > 3 files changed, 69 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
> > > index 8584f49..0e55e5c 100644
> > > --- a/tools/kvm/include/kvm/virtio-9p.h
> > > +++ b/tools/kvm/include/kvm/virtio-9p.h
> > > @@ -11,8 +11,8 @@
> > > #define VIRTQUEUE_NUM 128
> > > #define VIRTIO_P9_DEFAULT_TAG "kvm_9p"
> > > #define VIRTIO_P9_HDR_LEN (sizeof(u32)+sizeof(u8)+sizeof(u16))
> > > -#define VIRTIO_P9_MAX_FID 128
> > > -#define VIRTIO_P9_VERSION "9P2000"
> > > +#define VIRTIO_P9_MAX_FID 256
> > > +#define VIRTIO_P9_VERSION "9P2000.u"
> > > #define MAX_TAG_LEN 32
> > >
> > > struct p9_msg {
> > > diff --git a/tools/kvm/virtio/9p-pdu.c b/tools/kvm/virtio/9p-pdu.c
> > > index 0c454db..8ed249f 100644
> > > --- a/tools/kvm/virtio/9p-pdu.c
> > > +++ b/tools/kvm/virtio/9p-pdu.c
> > > @@ -200,13 +200,15 @@ static int virtio_p9_pdu_encode(struct p9_pdu *pdu, const char *fmt, va_list ap)
> > > case 'S':
> > > {
> > > struct p9_wstat *stbuf = va_arg(ap, struct p9_wstat *);
> > > - retval = virtio_p9_pdu_writef(pdu, "wwdQdddqssss",
> > > + retval = virtio_p9_pdu_writef(pdu, "wwdQdddqsssssddd",
> > > stbuf->size, stbuf->type,
> > > stbuf->dev, &stbuf->qid,
> > > stbuf->mode, stbuf->atime,
> > > stbuf->mtime, stbuf->length,
> > > stbuf->name, stbuf->uid,
> > > - stbuf->gid, stbuf->muid);
> > > + stbuf->gid, stbuf->muid,
> > > + stbuf->extension, stbuf->n_uid,
> > > + stbuf->n_gid, stbuf->n_muid);
> > > }
> > > break;
> > > default:
> > > diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> > > index 3b5555c..a6eafe0 100644
> > > --- a/tools/kvm/virtio/9p.c
> > > +++ b/tools/kvm/virtio/9p.c
> > > @@ -162,7 +162,7 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev,
> > >
> > > err_str = strerror(err);
> > > pdu->write_offset = VIRTIO_P9_HDR_LEN;
> > > - virtio_p9_pdu_writef(pdu, "s", err_str);
> > > + virtio_p9_pdu_writef(pdu, "sd", err_str, -err);
> >
> > Should that be -err ? I guess it should the errno value ?
>
> Yup, but usually we get -errno back from functions and pass it to
> virtio_p9_error_reply() while 9p expects it to be errno.
Most of the libc functions should return -1 on error and set errno right
? and i guess most of the 9p function caller virtio_p9_error_reply with
errno as argument
>
> >
> > > *outlen = pdu->write_offset;
> > >
> > > pdu->read_offset = sizeof(u32) + sizeof(u8);
> > > @@ -204,7 +204,6 @@ static void virtio_p9_open(struct p9_dev *p9dev,
> > > struct p9_qid qid;
> > > struct p9_fid *new_fid;
> > >
> > > -
> > > virtio_p9_pdu_readf(pdu, "db", &fid, &mode);
> > > new_fid = &p9dev->fids[fid];
> > >
> > > @@ -242,13 +241,14 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> > > u8 mode;
> > > u32 perm;
> > > char *name;
> > > + char *ext = NULL;
> > > u32 fid_val;
> > > struct stat st;
> > > struct p9_qid qid;
> > > struct p9_fid *fid;
> > > char full_path[PATH_MAX];
> > >
> > > - virtio_p9_pdu_readf(pdu, "dsdb", &fid_val, &name, &perm, &mode);
> > > + virtio_p9_pdu_readf(pdu, "dsdbs", &fid_val, &name, &perm, &mode,
> > > &ext);
> >
> > ext need to be freed. ? I guess we have other places where the memory
> > alloc did for readf(.., "s", ..) is not freed.
>
> Oh heh, I took care of the part where we create wstat and write it, not
> for the part where we read it :)
>
> >
> > > fid = &p9dev->fids[fid_val];
> > >
> > > sprintf(full_path, "%s/%s", fid->abs_path, name);
> > > @@ -262,6 +262,30 @@ static void virtio_p9_create(struct p9_dev *p9dev,
> > > close_fid(p9dev, fid_val);
> > > fid->dir = dir;
> > > fid->is_dir = 1;
> > > + } else if (perm & P9_DMSYMLINK) {
> > > + int r;
> > > +
> > > + r = symlink(ext, full_path);
> > > + if (r < 0)
> > > + goto err_out;
> > > + fd = open(full_path, omode2uflags(mode));
> > > + if (fd < 0)
> > > + goto err_out;
> > > + close_fid(p9dev, fid_val);
> > > + fid->fd = fd;
> >
> >
> > that updates the fid ? Is that needed ? Also above we are just updating
> > fid->fd where as fid->path still points to the old name. I guess
> > symlink and link don't result in open
>
> I'm really not sure if it's needed, the RFC isn't specific as to whether
> we open 'special' files after creation or not, so I wanted to go with
> opening them just in case (the kernel will clunk them anyway later, so
> what the hell...).
>
> Regarding the open in the symlink case, it's same deal really - I wasn't
> sure whether create opens the file or not, and since theres no point in
> opening a symlink I went ahead and opened the target of the symlink.
>
> I didn't see a case where the kernel uses a linked/symlinked file
> straight away, but as the RFC wasn't specific it was implemented anyway.
>
For the linux client we don't need the file to be open. There are few
issues here.
1) You are only updating fid->fd, But fid->path is still the old path
2) You are following symlink on host. where as it should actually be
followed on the guest. (for ex: if we do ln -s /etc/a k on guest, we
actually want to open /etc/a on guest not the one on host.)
> >
> > > + } else if (perm & P9_DMLINK) {
> > > + int r;
> > > + int ext_fid = atoi(ext);
> > > +
> > > + r = link(p9dev->fids[ext_fid].abs_path, full_path);
> > > + if (r < 0)
> > > + goto err_out;
> > > +
> > > + fd = open(full_path, omode2uflags(mode));
> > > + if (fd < 0)
> > > + goto err_out;
> > > + close_fid(p9dev, fid_val);
> > > + fid->fd = fd;
> > > } else {
> > > fd = open(full_path, omode2uflags(mode) | O_CREAT, 0777);
> > > if (fd < 0)
> > > @@ -294,7 +318,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > > u32 newfid_val;
> > > struct p9_qid wqid;
> > > struct p9_fid *new_fid;
> > > -
> > > + int ret;
> > >
> > > virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
> > > new_fid = &p9dev->fids[newfid_val];
> > > @@ -315,8 +339,10 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > > /* Format the new path we're 'walk'ing into */
> > > sprintf(tmp, "%s/%.*s",
> > > fid->path, (int)strlen(str), str);
> > > - if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0)
> > > + if (lstat(rel_to_abs(p9dev, tmp, full_path), &st) < 0) {
> > > + ret = ENOENT;
> > > goto err_out;
> > > + }
> >
> > Why ?. What error with lstat failed which we wanted to map to ENOENT ?
>
> stat of non-existent files.
>
Won't that return ENOENT ?
lstat("k", 0x7fffd5d350c0) = -1 ENOENT (No such file or directory)
> >
> > >
> > > st2qid(&st, &wqid);
> > > new_fid->is_dir = S_ISDIR(st.st_mode);
> > > @@ -340,7 +366,7 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
> > > virtio_p9_set_reply_header(pdu, *outlen);
> > > return;
> > > err_out:
> > > - virtio_p9_error_reply(p9dev, pdu, errno, outlen);
> > > + virtio_p9_error_reply(p9dev, pdu, ret, outlen);
> > > return;
> > > }
-aneesh
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-08-02 16:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-01 14:08 [PATCH 1/2] kvm tools: Add support for 9p2000.u Sasha Levin
2011-08-01 14:08 ` [PATCH 2/2] kvm tools: Add '--rootfs' and '--binsh' Sasha Levin
2011-08-01 15:12 ` [PATCH 1/2] kvm tools: Add support for 9p2000.u Aneesh Kumar K.V
2011-08-01 15:26 ` Sasha Levin
2011-08-02 16:36 ` Aneesh Kumar K.V
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox