Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 10:46, Fam Zheng wrote:
On 2013年12月17日 17:07, Peter Lieven wrote:
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors *
BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async
call.
On open I fstat anyway and store that value. For qemu-img info
this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before
accessing
filename[6].
Good point. I will check for this, but in fact I think it can't
happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags,
&client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK descri
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 17:07, Peter Lieven wrote:
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors *
BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async
call.
On open I fstat anyway and store that value. For qemu-img info
this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before
accessing
filename[6].
Good point. I will check for this, but in fact I think it can't
happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags,
&client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please a
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags,
&client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't r
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags,
&client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add che
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags, &client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read be
Re: [Qemu-devel] [PATCH] block: add native support for NFS
于2013年12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh)
!= 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags, &client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read beyond EOF as well.
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write. -ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive, especially with
strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh) != 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags, &client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential for VMDK
description file. But you are right, please add check code and make sure that
we don't read beyond EOF as well.
Actually it would like to keep bs->total
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs->opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, &Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client->context, client->fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, &Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs->total_sectors = MAX(bs->total_sectors, sector_num +
nb_sectors);
+client->allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write. -ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs->opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(&runtime_opts);
+qemu_opts_absorb_qdict(opts, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, "filename");
+
+client->context = nfs_init_context();
+
+if (client->context == NULL) {
+error_setg(errp, "Failed to init NFS context");
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive, especially
with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, "Invalid server in URL");
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, "Invalid URL specified.\n");
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client->context, server, path) != 0) {
+error_setg(errp, "Failed to mount nfs share: %s",
+nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags & O_CREAT) {
+if (nfs_creat(client->context, file, 0600, &client->fh) != 0) {
+error_setg(errp, "Failed to create file: %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client->context, file, open_flags, &client->fh)
!= 0) {
+error_setg(errp, "Failed to open file : %s",
+ nfs_get_error(client->context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client->context, client->fh, &st) != 0) {
+error_setg(errp, "Failed to fstat file: %s",
+ nfs_get_error(client->context));
+ret = -EIO;
+goto fail;
+}
+
+bs->total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential for
VMDK description file. But you are right, please add check code and make
sure that we don't read beyond EOF as well.
Thanks for reviewing!
One wish as I think you are the VMDK format maintaine
Re: [Qemu-devel] [PATCH] block: add native support for NFS
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
This patch adds native support for accessing images on NFS shares without
the requirement to actually mount the entire NFS share on the host.
NFS Images can simply be specified by an url of the form:
nfs:
For example:
qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
You need libnfs from Ronnie Sahlberg available at:
git://github.com/sahlberg/libnfs.git
for this to work.
During configure it is automatically probed for libnfs and support
is enabled on-the-fly. You can forbid or enforce libnfs support
with --disable-libnfs or --enable-libnfs respectively.
Due to NFS restrictions you might need to execute your binaries
as root, allow them to open priviledged ports (<1024) or specify
insecure option on the NFS server.
Signed-off-by: Peter Lieven
Looks nice! Thanks for the work!
Thank you ;-)
---
MAINTAINERS |5 +
block/Makefile.objs |1 +
block/nfs.c | 420 +++
configure | 38 +
4 files changed, 464 insertions(+)
create mode 100644 block/nfs.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c19133f..f53d184 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -899,6 +899,11 @@ M: Peter Lieven
S: Supported
F: block/iscsi.c
+NFS
+M: Peter Lieven
+S: Maintained
+F: block/nfs.c
+
SSH
M: Richard W.M. Jones
S: Supported
diff --git a/block/Makefile.objs b/block/Makefile.objs
index f43ecbc..1bac94e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
+block-obj-$(CONFIG_LIBISCSI) += nfs.o
block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/nfs.c b/block/nfs.c
new file mode 100644
index 000..d6cb4c0
--- /dev/null
+++ b/block/nfs.c
@@ -0,0 +1,420 @@
+/*
+ * QEMU Block driver for native access to files on NFS shares
+ *
+ * Copyright (c) 2013 Peter Lieven
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include
+#include
+#include "qemu-common.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "block/block_int.h"
+#include "trace.h"
+#include "block/scsi.h"
+#include "qemu/iov.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
Copied from block/iscsi.c? SCSI and QMP are not necessary for this file. And
maybe also arpa/inet.h, I'm not sure about that though.
Yes, libiscsi and libnfs are quite similar. ;-)
+
+#include
+#include
+#include
+#include
+
+typedef struct nfsclient {
+struct nfs_context *context;
+struct nfsfh *fh;
+int events;
+bool has_zero_init;
+int64_t allocated_file_size;
+QEMUBH *close_bh;
This is unused.
Ups, its a leftover from a nasty segfault debug session.
+} nfsclient;
Please use CamelCase for type names...
Ok
+
+typedef struct NFSTask {
+int status;
+int complete;
+QEMUIOVector *iov;
+Coroutine *co;
+QEMUBH *bh;
+} NFSTask;
as you do with this.
+
+static void nfs_process_read(void *arg);
+static void nfs_process_write(void *arg);
+
+static void nfs_set_events(nfsclient *client)
+{
+int ev;
+/* We always register a read handler. */
+ev = POLLIN;
+ev |= nfs_which_events(client->context);
+if (ev != client->events) {
+qemu_aio_set_fd_handler(nfs_get_fd(client->context),
+ nfs_process_read,
+ (ev & POLLOUT) ? nfs_process_write : NULL,
+ client);
+
+}
+client->events = ev;
+}
+
+static void nfs_process_read(void *arg)
+{
+nfsclient *client = arg;
+nfs_service(client->context, POLLIN);
+nfs_set_events(client);
+}
+
+static void nfs_process_write(void
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月16日 23:34, Peter Lieven wrote:
This patch adds native support for accessing images on NFS shares without
the requirement to actually mount the entire NFS share on the host.
NFS Images can simply be specified by an url of the form:
nfs:
For example:
qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
You need libnfs from Ronnie Sahlberg available at:
git://github.com/sahlberg/libnfs.git
for this to work.
During configure it is automatically probed for libnfs and support
is enabled on-the-fly. You can forbid or enforce libnfs support
with --disable-libnfs or --enable-libnfs respectively.
Due to NFS restrictions you might need to execute your binaries
as root, allow them to open priviledged ports (<1024) or specify
insecure option on the NFS server.
Signed-off-by: Peter Lieven
Looks nice! Thanks for the work!
---
MAINTAINERS |5 +
block/Makefile.objs |1 +
block/nfs.c | 420 +++
configure | 38 +
4 files changed, 464 insertions(+)
create mode 100644 block/nfs.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c19133f..f53d184 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -899,6 +899,11 @@ M: Peter Lieven
S: Supported
F: block/iscsi.c
+NFS
+M: Peter Lieven
+S: Maintained
+F: block/nfs.c
+
SSH
M: Richard W.M. Jones
S: Supported
diff --git a/block/Makefile.objs b/block/Makefile.objs
index f43ecbc..1bac94e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
+block-obj-$(CONFIG_LIBISCSI) += nfs.o
block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/nfs.c b/block/nfs.c
new file mode 100644
index 000..d6cb4c0
--- /dev/null
+++ b/block/nfs.c
@@ -0,0 +1,420 @@
+/*
+ * QEMU Block driver for native access to files on NFS shares
+ *
+ * Copyright (c) 2013 Peter Lieven
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include
+#include
+#include "qemu-common.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "block/block_int.h"
+#include "trace.h"
+#include "block/scsi.h"
+#include "qemu/iov.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
Copied from block/iscsi.c? SCSI and QMP are not necessary for this file.
And maybe also arpa/inet.h, I'm not sure about that though.
+
+#include
+#include
+#include
+#include
+
+typedef struct nfsclient {
+struct nfs_context *context;
+struct nfsfh *fh;
+int events;
+bool has_zero_init;
+int64_t allocated_file_size;
+QEMUBH *close_bh;
This is unused.
+} nfsclient;
Please use CamelCase for type names...
+
+typedef struct NFSTask {
+int status;
+int complete;
+QEMUIOVector *iov;
+Coroutine *co;
+QEMUBH *bh;
+} NFSTask;
as you do with this.
+
+static void nfs_process_read(void *arg);
+static void nfs_process_write(void *arg);
+
+static void nfs_set_events(nfsclient *client)
+{
+int ev;
+/* We always register a read handler. */
+ev = POLLIN;
+ev |= nfs_which_events(client->context);
+if (ev != client->events) {
+qemu_aio_set_fd_handler(nfs_get_fd(client->context),
+ nfs_process_read,
+ (ev & POLLOUT) ? nfs_process_write : NULL,
+ client);
+
+}
+client->events = ev;
+}
+
+static void nfs_process_read(void *arg)
+{
+nfsclient *client = arg;
+nfs_service(client->context, POLLIN);
+nfs_set_events(client);
+}
+
+static void nfs_process_write(void *arg)
+{
+nfsclient *client = arg;
+nfs_service(client->context, POLLOUT);
+nfs_set_events(client);
+}
+
+static void nfs_co_init_task(nfsclient *client, NFSTask *Ta
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On Mon, Dec 16, 2013 at 7:34 AM, Peter Lieven wrote:
> This patch adds native support for accessing images on NFS shares without
> the requirement to actually mount the entire NFS share on the host.
>
> NFS Images can simply be specified by an url of the form:
> nfs:
>
> For example:
> qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
>
> You need libnfs from Ronnie Sahlberg available at:
>git://github.com/sahlberg/libnfs.git
> for this to work.
>
> During configure it is automatically probed for libnfs and support
> is enabled on-the-fly. You can forbid or enforce libnfs support
> with --disable-libnfs or --enable-libnfs respectively.
>
> Due to NFS restrictions you might need to execute your binaries
> as root, allow them to open priviledged ports (<1024) or specify
> insecure option on the NFS server.
>
> Signed-off-by: Peter Lieven
> ---
> MAINTAINERS |5 +
> block/Makefile.objs |1 +
> block/nfs.c | 420
> +++
> configure | 38 +
> 4 files changed, 464 insertions(+)
> create mode 100644 block/nfs.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c19133f..f53d184 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -899,6 +899,11 @@ M: Peter Lieven
> S: Supported
> F: block/iscsi.c
>
> +NFS
> +M: Peter Lieven
> +S: Maintained
> +F: block/nfs.c
> +
> SSH
> M: Richard W.M. Jones
> S: Supported
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index f43ecbc..1bac94e 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> ifeq ($(CONFIG_POSIX),y)
> block-obj-y += nbd.o sheepdog.o
> block-obj-$(CONFIG_LIBISCSI) += iscsi.o
> +block-obj-$(CONFIG_LIBISCSI) += nfs.o
CONFIG_LIBNFS
> block-obj-$(CONFIG_CURL) += curl.o
> block-obj-$(CONFIG_RBD) += rbd.o
> block-obj-$(CONFIG_GLUSTERFS) += gluster.o
> diff --git a/block/nfs.c b/block/nfs.c
> new file mode 100644
> index 000..d6cb4c0
> --- /dev/null
> +++ b/block/nfs.c
> @@ -0,0 +1,420 @@
> +/*
> + * QEMU Block driver for native access to files on NFS shares
> + *
> + * Copyright (c) 2013 Peter Lieven
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "config-host.h"
> +
> +#include
> +#include
> +#include "qemu-common.h"
> +#include "qemu/config-file.h"
> +#include "qemu/error-report.h"
> +#include "block/block_int.h"
> +#include "trace.h"
> +#include "block/scsi.h"
> +#include "qemu/iov.h"
> +#include "sysemu/sysemu.h"
> +#include "qmp-commands.h"
> +
> +#include
> +#include
> +#include
> +#include
> +
> +typedef struct nfsclient {
> +struct nfs_context *context;
> +struct nfsfh *fh;
> +int events;
> +bool has_zero_init;
> +int64_t allocated_file_size;
> +QEMUBH *close_bh;
> +} nfsclient;
> +
> +typedef struct NFSTask {
> +int status;
> +int complete;
> +QEMUIOVector *iov;
> +Coroutine *co;
> +QEMUBH *bh;
> +} NFSTask;
> +
> +static void nfs_process_read(void *arg);
> +static void nfs_process_write(void *arg);
> +
> +static void nfs_set_events(nfsclient *client)
> +{
> +int ev;
> +/* We always register a read handler. */
> +ev = POLLIN;
I don't think you should always register a read handler here since
there are no situations where the NFS server
will send unsolicited packets back to the client (contrary to iSCSI
where the target might send NOPs back to ping the client).
When NFS sessions are idle, it is common that servers will tear down
the TCP connection to save resources and rely on that
the clients will transparently reconnect the TPC session again once
there is new I/O to send.
This reconnect is handled automatically/transparently within libnfs.
> +ev |= nfs_which_events(client->context);
> +if (ev != client->events) {
> +qemu_aio_set_fd_handler(nfs_get_fd(client->contex
Re: [Qemu-devel] [PATCH] block: add native support for NFS
Nice.
+block-obj-$(CONFIG_LIBISCSI) += nfs.
Should be CONFIG_LIBNFS
On Mon, Dec 16, 2013 at 7:34 AM, Peter Lieven wrote:
> This patch adds native support for accessing images on NFS shares without
> the requirement to actually mount the entire NFS share on the host.
>
> NFS Images can simply be specified by an url of the form:
> nfs:
>
> For example:
> qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
>
> You need libnfs from Ronnie Sahlberg available at:
>git://github.com/sahlberg/libnfs.git
> for this to work.
>
> During configure it is automatically probed for libnfs and support
> is enabled on-the-fly. You can forbid or enforce libnfs support
> with --disable-libnfs or --enable-libnfs respectively.
>
> Due to NFS restrictions you might need to execute your binaries
> as root, allow them to open priviledged ports (<1024) or specify
> insecure option on the NFS server.
>
> Signed-off-by: Peter Lieven
> ---
> MAINTAINERS |5 +
> block/Makefile.objs |1 +
> block/nfs.c | 420
> +++
> configure | 38 +
> 4 files changed, 464 insertions(+)
> create mode 100644 block/nfs.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c19133f..f53d184 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -899,6 +899,11 @@ M: Peter Lieven
> S: Supported
> F: block/iscsi.c
>
> +NFS
> +M: Peter Lieven
> +S: Maintained
> +F: block/nfs.c
> +
> SSH
> M: Richard W.M. Jones
> S: Supported
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index f43ecbc..1bac94e 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> ifeq ($(CONFIG_POSIX),y)
> block-obj-y += nbd.o sheepdog.o
> block-obj-$(CONFIG_LIBISCSI) += iscsi.o
> +block-obj-$(CONFIG_LIBISCSI) += nfs.o
> block-obj-$(CONFIG_CURL) += curl.o
> block-obj-$(CONFIG_RBD) += rbd.o
> block-obj-$(CONFIG_GLUSTERFS) += gluster.o
> diff --git a/block/nfs.c b/block/nfs.c
> new file mode 100644
> index 000..d6cb4c0
> --- /dev/null
> +++ b/block/nfs.c
> @@ -0,0 +1,420 @@
> +/*
> + * QEMU Block driver for native access to files on NFS shares
> + *
> + * Copyright (c) 2013 Peter Lieven
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "config-host.h"
> +
> +#include
> +#include
> +#include "qemu-common.h"
> +#include "qemu/config-file.h"
> +#include "qemu/error-report.h"
> +#include "block/block_int.h"
> +#include "trace.h"
> +#include "block/scsi.h"
> +#include "qemu/iov.h"
> +#include "sysemu/sysemu.h"
> +#include "qmp-commands.h"
> +
> +#include
> +#include
> +#include
> +#include
> +
> +typedef struct nfsclient {
> +struct nfs_context *context;
> +struct nfsfh *fh;
> +int events;
> +bool has_zero_init;
> +int64_t allocated_file_size;
> +QEMUBH *close_bh;
> +} nfsclient;
> +
> +typedef struct NFSTask {
> +int status;
> +int complete;
> +QEMUIOVector *iov;
> +Coroutine *co;
> +QEMUBH *bh;
> +} NFSTask;
> +
> +static void nfs_process_read(void *arg);
> +static void nfs_process_write(void *arg);
> +
> +static void nfs_set_events(nfsclient *client)
> +{
> +int ev;
> +/* We always register a read handler. */
> +ev = POLLIN;
> +ev |= nfs_which_events(client->context);
> +if (ev != client->events) {
> +qemu_aio_set_fd_handler(nfs_get_fd(client->context),
> + nfs_process_read,
> + (ev & POLLOUT) ? nfs_process_write : NULL,
> + client);
> +
> +}
> +client->events = ev;
> +}
> +
> +static void nfs_process_read(void *arg)
> +{
> +nfsclient *client = arg;
> +nfs_service(client->context, POLLIN);
> +nfs_set_events(client);
> +}
> +
> +static void nfs_process_write(void *arg)
> +{
> +nfsclient *client = arg;
> +nfs_service(client->c
[Qemu-devel] [PATCH] block: add native support for NFS
This patch adds native support for accessing images on NFS shares without
the requirement to actually mount the entire NFS share on the host.
NFS Images can simply be specified by an url of the form:
nfs:
For example:
qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
You need libnfs from Ronnie Sahlberg available at:
git://github.com/sahlberg/libnfs.git
for this to work.
During configure it is automatically probed for libnfs and support
is enabled on-the-fly. You can forbid or enforce libnfs support
with --disable-libnfs or --enable-libnfs respectively.
Due to NFS restrictions you might need to execute your binaries
as root, allow them to open priviledged ports (<1024) or specify
insecure option on the NFS server.
Signed-off-by: Peter Lieven
---
MAINTAINERS |5 +
block/Makefile.objs |1 +
block/nfs.c | 420 +++
configure | 38 +
4 files changed, 464 insertions(+)
create mode 100644 block/nfs.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c19133f..f53d184 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -899,6 +899,11 @@ M: Peter Lieven
S: Supported
F: block/iscsi.c
+NFS
+M: Peter Lieven
+S: Maintained
+F: block/nfs.c
+
SSH
M: Richard W.M. Jones
S: Supported
diff --git a/block/Makefile.objs b/block/Makefile.objs
index f43ecbc..1bac94e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
+block-obj-$(CONFIG_LIBISCSI) += nfs.o
block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/nfs.c b/block/nfs.c
new file mode 100644
index 000..d6cb4c0
--- /dev/null
+++ b/block/nfs.c
@@ -0,0 +1,420 @@
+/*
+ * QEMU Block driver for native access to files on NFS shares
+ *
+ * Copyright (c) 2013 Peter Lieven
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include
+#include
+#include "qemu-common.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "block/block_int.h"
+#include "trace.h"
+#include "block/scsi.h"
+#include "qemu/iov.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
+
+#include
+#include
+#include
+#include
+
+typedef struct nfsclient {
+struct nfs_context *context;
+struct nfsfh *fh;
+int events;
+bool has_zero_init;
+int64_t allocated_file_size;
+QEMUBH *close_bh;
+} nfsclient;
+
+typedef struct NFSTask {
+int status;
+int complete;
+QEMUIOVector *iov;
+Coroutine *co;
+QEMUBH *bh;
+} NFSTask;
+
+static void nfs_process_read(void *arg);
+static void nfs_process_write(void *arg);
+
+static void nfs_set_events(nfsclient *client)
+{
+int ev;
+/* We always register a read handler. */
+ev = POLLIN;
+ev |= nfs_which_events(client->context);
+if (ev != client->events) {
+qemu_aio_set_fd_handler(nfs_get_fd(client->context),
+ nfs_process_read,
+ (ev & POLLOUT) ? nfs_process_write : NULL,
+ client);
+
+}
+client->events = ev;
+}
+
+static void nfs_process_read(void *arg)
+{
+nfsclient *client = arg;
+nfs_service(client->context, POLLIN);
+nfs_set_events(client);
+}
+
+static void nfs_process_write(void *arg)
+{
+nfsclient *client = arg;
+nfs_service(client->context, POLLOUT);
+nfs_set_events(client);
+}
+
+static void nfs_co_init_task(nfsclient *client, NFSTask *Task)
+{
+*Task = (NFSTask) {
+.co = qemu_coroutine_self(),
+};
+}
+
+static void nfs_co_generic_bh_cb(void *opaque)
+{
+NFSTask *Task = opaque;
+qemu_bh_delete(Task->bh);
+qemu_coroutine_enter(Task->co, NULL);
+}
+
+static void nfs_co_generic_cb(int status, struct nfs_context *nfs, void *d
