Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 11/03/2015 09:55 PM, Paolo Bonzini wrote: On 03/11/2015 04:56, Xiao Guangrong wrote: On 11/03/2015 05:12 AM, Paolo Bonzini wrote: On 02/11/2015 10:13, Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { I was going to send tomorrow a pull request for a similar patch, "backends/hostmem-file: Allow to specify full pathname for backing file". The main difference seems to be your usage of O_EXCL. Can you explain why you added it? It' used if we pass a block device as a NVDIMM backend memory: O_EXCL can be used without O_CREAT if pathname refers to a block device. If the block device is in use by the system (e.g., mounted), open() fails with the error EBUSY That makes sense, but I think it's better to be consistent with the handling of block devices. Block devices do not use O_EXCL when QEMU opens them; I guess in principle it would also be possible to share a single pmem backend between multiple guests. Yup. Will make a separate patch to do this. :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 03/11/2015 04:56, Xiao Guangrong wrote: > > > On 11/03/2015 05:12 AM, Paolo Bonzini wrote: >> >> >> On 02/11/2015 10:13, Xiao Guangrong wrote: >>> Currently, file_ram_alloc() only works on directory - it creates a file >>> under @path and do mmap on it >>> >>> This patch tries to allow it to work on file directly, if @path is a >>> directory it works as before, otherwise it treats @path as the target >>> file then directly allocate memory from it >>> >>> Signed-off-by: Xiao Guangrong >>> --- >>> exec.c | 80 >>> ++ >>> 1 file changed, 51 insertions(+), 29 deletions(-) >>> >>> diff --git a/exec.c b/exec.c >>> index 9075f4d..db0fdaf 100644 >>> --- a/exec.c >>> +++ b/exec.c >>> @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) >>> } >>> >>> #ifdef __linux__ >>> +static bool path_is_dir(const char *path) >>> +{ >>> +struct stat fs; >>> + >>> +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); >>> +} >>> + >>> +static int open_ram_file_path(RAMBlock *block, const char *path, >>> size_t size) >>> +{ >>> +char *filename; >>> +char *sanitized_name; >>> +char *c; >>> +int fd; >>> + >>> +if (!path_is_dir(path)) { >>> +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; >>> + >>> +flags |= O_EXCL; >>> +return open(path, flags); >>> +} >>> + >>> +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ >>> +sanitized_name = g_strdup(memory_region_name(block->mr)); >>> +for (c = sanitized_name; *c != '\0'; c++) { >>> +if (*c == '/') { >>> +*c = '_'; >>> +} >>> +} >>> +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, >>> + sanitized_name); >>> +g_free(sanitized_name); >>> +fd = mkstemp(filename); >>> +if (fd >= 0) { >>> +unlink(filename); >>> +/* >>> + * ftruncate is not supported by hugetlbfs in older >>> + * hosts, so don't bother bailing out on errors. >>> + * If anything goes wrong with it under other filesystems, >>> + * mmap will fail. >>> + */ >>> +if (ftruncate(fd, size)) { >>> +perror("ftruncate"); >>> +} >>> +} >>> +g_free(filename); >>> + >>> +return fd; >>> +} >>> + >>> static void *file_ram_alloc(RAMBlock *block, >>> ram_addr_t memory, >>> const char *path, >>> Error **errp) >>> { >>> -char *filename; >>> -char *sanitized_name; >>> -char *c; >>> void *area; >>> int fd; >>> uint64_t pagesize; >>> @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, >>> goto error; >>> } >>> >>> -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ >>> -sanitized_name = g_strdup(memory_region_name(block->mr)); >>> -for (c = sanitized_name; *c != '\0'; c++) { >>> -if (*c == '/') >>> -*c = '_'; >>> -} >>> - >>> -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, >>> - sanitized_name); >>> -g_free(sanitized_name); >>> +memory = ROUND_UP(memory, pagesize); >>> >>> -fd = mkstemp(filename); >>> +fd = open_ram_file_path(block, path, memory); >>> if (fd < 0) { >>> error_setg_errno(errp, errno, >>>"unable to create backing store for path >>> %s", path); >>> -g_free(filename); >>> goto error; >>> } >>> -unlink(filename); >>> -g_free(filename); >>> - >>> -memory = ROUND_UP(memory, pagesize); >>> - >>> -/* >>> - * ftruncate is not supported by hugetlbfs in older >>> - * hosts, so don't bother bailing out on errors. >>> - * If anything goes wrong with it under other filesystems, >>> - * mmap will fail. >>> - */ >>> -if (ftruncate(fd, memory)) { >>> -perror("ftruncate"); >>> -} >>> >>> area = qemu_ram_mmap(fd, memory, pagesize, block->flags & >>> RAM_SHARED); >>> if (area == MAP_FAILED) { >>> >> >> I was going to send tomorrow a pull request for a similar patch, >> "backends/hostmem-file: Allow to specify full pathname for backing file". >> >> The main difference seems to be your usage of O_EXCL. Can you explain >> why you added it? > > It' used if we pass a block device as a NVDIMM backend memory: > O_EXCL can be used without O_CREAT if pathname refers to a block > device. If the block device > is in use by the system (e.g., mounted), open() fails with the error EBUSY That makes sense, but I think it's better to be consistent with the handling of block devices. Block devices do not use O_EXCL when QEMU opens them; I guess in principle it would also be possible to share a single pmem backend between multiple guests. Paolo -- To unsubscribe from this list: send the line "unsubscr
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 11/03/2015 08:34 PM, Igor Mammedov wrote: On Mon, 2 Nov 2015 17:13:11 +0800 Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Paolo has just queued https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06513.html perhaps that's what you can reuse here. Yep, Paolo has told me about that, i will update this patchset after his pull request. BTW, which tree should this patchset be based on in future development? Paolo's or Michael's or even upstream qemu tree? Thanks! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On Mon, 2 Nov 2015 17:13:11 +0800 Xiao Guangrong wrote: > Currently, file_ram_alloc() only works on directory - it creates a file > under @path and do mmap on it > > This patch tries to allow it to work on file directly, if @path is a > directory it works as before, otherwise it treats @path as the target > file then directly allocate memory from it Paolo has just queued https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06513.html perhaps that's what you can reuse here. > > Signed-off-by: Xiao Guangrong > --- > exec.c | 80 > ++ > 1 file changed, 51 insertions(+), 29 deletions(-) > > diff --git a/exec.c b/exec.c > index 9075f4d..db0fdaf 100644 > --- a/exec.c > +++ b/exec.c > @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) > } > > #ifdef __linux__ > +static bool path_is_dir(const char *path) > +{ > +struct stat fs; > + > +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); > +} > + > +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) > +{ > +char *filename; > +char *sanitized_name; > +char *c; > +int fd; > + > +if (!path_is_dir(path)) { > +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; > + > +flags |= O_EXCL; > +return open(path, flags); > +} > + > +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ > +sanitized_name = g_strdup(memory_region_name(block->mr)); > +for (c = sanitized_name; *c != '\0'; c++) { > +if (*c == '/') { > +*c = '_'; > +} > +} > +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, > + sanitized_name); > +g_free(sanitized_name); > +fd = mkstemp(filename); > +if (fd >= 0) { > +unlink(filename); > +/* > + * ftruncate is not supported by hugetlbfs in older > + * hosts, so don't bother bailing out on errors. > + * If anything goes wrong with it under other filesystems, > + * mmap will fail. > + */ > +if (ftruncate(fd, size)) { > +perror("ftruncate"); > +} > +} > +g_free(filename); > + > +return fd; > +} > + > static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > const char *path, > Error **errp) > { > -char *filename; > -char *sanitized_name; > -char *c; > void *area; > int fd; > uint64_t pagesize; > @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, > goto error; > } > > -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ > -sanitized_name = g_strdup(memory_region_name(block->mr)); > -for (c = sanitized_name; *c != '\0'; c++) { > -if (*c == '/') > -*c = '_'; > -} > - > -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, > - sanitized_name); > -g_free(sanitized_name); > +memory = ROUND_UP(memory, pagesize); > > -fd = mkstemp(filename); > +fd = open_ram_file_path(block, path, memory); > if (fd < 0) { > error_setg_errno(errp, errno, > "unable to create backing store for path %s", path); > -g_free(filename); > goto error; > } > -unlink(filename); > -g_free(filename); > - > -memory = ROUND_UP(memory, pagesize); > - > -/* > - * ftruncate is not supported by hugetlbfs in older > - * hosts, so don't bother bailing out on errors. > - * If anything goes wrong with it under other filesystems, > - * mmap will fail. > - */ > -if (ftruncate(fd, memory)) { > -perror("ftruncate"); > -} > > area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); > if (area == MAP_FAILED) { -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 11/03/2015 05:12 AM, Paolo Bonzini wrote: On 02/11/2015 10:13, Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { I was going to send tomorrow a pull request for a similar patch, "backends/hostmem-file: Allow to specify full pathname for backing file". The main difference seems to be your usage of O_EXCL. Can you explain why you added it? It' used if we pass a block device as a NVDIMM backend memory: O_EXCL can be used without O_CREAT if pathname refers to a block device. If the block device is in use by the system (e.g., mounted), open() fails with the error EBUSY -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 02/11/2015 10:13, Xiao Guangrong wrote: > Currently, file_ram_alloc() only works on directory - it creates a file > under @path and do mmap on it > > This patch tries to allow it to work on file directly, if @path is a > directory it works as before, otherwise it treats @path as the target > file then directly allocate memory from it > > Signed-off-by: Xiao Guangrong > --- > exec.c | 80 > ++ > 1 file changed, 51 insertions(+), 29 deletions(-) > > diff --git a/exec.c b/exec.c > index 9075f4d..db0fdaf 100644 > --- a/exec.c > +++ b/exec.c > @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) > } > > #ifdef __linux__ > +static bool path_is_dir(const char *path) > +{ > +struct stat fs; > + > +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); > +} > + > +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) > +{ > +char *filename; > +char *sanitized_name; > +char *c; > +int fd; > + > +if (!path_is_dir(path)) { > +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; > + > +flags |= O_EXCL; > +return open(path, flags); > +} > + > +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ > +sanitized_name = g_strdup(memory_region_name(block->mr)); > +for (c = sanitized_name; *c != '\0'; c++) { > +if (*c == '/') { > +*c = '_'; > +} > +} > +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, > + sanitized_name); > +g_free(sanitized_name); > +fd = mkstemp(filename); > +if (fd >= 0) { > +unlink(filename); > +/* > + * ftruncate is not supported by hugetlbfs in older > + * hosts, so don't bother bailing out on errors. > + * If anything goes wrong with it under other filesystems, > + * mmap will fail. > + */ > +if (ftruncate(fd, size)) { > +perror("ftruncate"); > +} > +} > +g_free(filename); > + > +return fd; > +} > + > static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > const char *path, > Error **errp) > { > -char *filename; > -char *sanitized_name; > -char *c; > void *area; > int fd; > uint64_t pagesize; > @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, > goto error; > } > > -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ > -sanitized_name = g_strdup(memory_region_name(block->mr)); > -for (c = sanitized_name; *c != '\0'; c++) { > -if (*c == '/') > -*c = '_'; > -} > - > -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, > - sanitized_name); > -g_free(sanitized_name); > +memory = ROUND_UP(memory, pagesize); > > -fd = mkstemp(filename); > +fd = open_ram_file_path(block, path, memory); > if (fd < 0) { > error_setg_errno(errp, errno, > "unable to create backing store for path %s", path); > -g_free(filename); > goto error; > } > -unlink(filename); > -g_free(filename); > - > -memory = ROUND_UP(memory, pagesize); > - > -/* > - * ftruncate is not supported by hugetlbfs in older > - * hosts, so don't bother bailing out on errors. > - * If anything goes wrong with it under other filesystems, > - * mmap will fail. > - */ > -if (ftruncate(fd, memory)) { > -perror("ftruncate"); > -} > > area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); > if (area == MAP_FAILED) { > I was going to send tomorrow a pull request for a similar patch, "backends/hostmem-file: Allow to specify full pathname for backing file". The main difference seems to be your usage of O_EXCL. Can you explain why you added it? Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 02.11.2015 18:25, Xiao Guangrong wrote: On 11/02/2015 11:12 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 12:13, Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a It isn't try to allow, it allows, as I understand)... Err... Sorry for my English, but what is the different between: ”This patch tries to allow it to work on file directly“ and "This patch allows it to work on file directly" :( Not sure that everyone is interested in this nit-picking discussion.. A allows B: if A then B A tries to allow B: if A then _may be_ B In any way it doesn't matter. directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); one empty line will be very nice here, and it was in master branch +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { Reviewed-by: Vladimir Sementsov-Ogievskiy Thanks for your review. -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 11/02/2015 11:12 PM, Vladimir Sementsov-Ogievskiy wrote: On 02.11.2015 12:13, Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a It isn't try to allow, it allows, as I understand)... Err... Sorry for my English, but what is the different between: ”This patch tries to allow it to work on file directly“ and "This patch allows it to work on file directly" :( directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); one empty line will be very nice here, and it was in master branch +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { Reviewed-by: Vladimir Sementsov-Ogievskiy Thanks for your review. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 09/35] exec: allow file_ram_alloc to work on file
On 02.11.2015 12:13, Xiao Guangrong wrote: Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a It isn't try to allow, it allows, as I understand) directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); one empty line will be very nice here, and it was in master branch +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 09/35] exec: allow file_ram_alloc to work on file
Currently, file_ram_alloc() only works on directory - it creates a file under @path and do mmap on it This patch tries to allow it to work on file directly, if @path is a directory it works as before, otherwise it treats @path as the target file then directly allocate memory from it Signed-off-by: Xiao Guangrong --- exec.c | 80 ++ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/exec.c b/exec.c index 9075f4d..db0fdaf 100644 --- a/exec.c +++ b/exec.c @@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void) } #ifdef __linux__ +static bool path_is_dir(const char *path) +{ +struct stat fs; + +return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode); +} + +static int open_ram_file_path(RAMBlock *block, const char *path, size_t size) +{ +char *filename; +char *sanitized_name; +char *c; +int fd; + +if (!path_is_dir(path)) { +int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY; + +flags |= O_EXCL; +return open(path, flags); +} + +/* Make name safe to use with mkstemp by replacing '/' with '_'. */ +sanitized_name = g_strdup(memory_region_name(block->mr)); +for (c = sanitized_name; *c != '\0'; c++) { +if (*c == '/') { +*c = '_'; +} +} +filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, + sanitized_name); +g_free(sanitized_name); +fd = mkstemp(filename); +if (fd >= 0) { +unlink(filename); +/* + * ftruncate is not supported by hugetlbfs in older + * hosts, so don't bother bailing out on errors. + * If anything goes wrong with it under other filesystems, + * mmap will fail. + */ +if (ftruncate(fd, size)) { +perror("ftruncate"); +} +} +g_free(filename); + +return fd; +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path, Error **errp) { -char *filename; -char *sanitized_name; -char *c; void *area; int fd; uint64_t pagesize; @@ -1212,38 +1258,14 @@ static void *file_ram_alloc(RAMBlock *block, goto error; } -/* Make name safe to use with mkstemp by replacing '/' with '_'. */ -sanitized_name = g_strdup(memory_region_name(block->mr)); -for (c = sanitized_name; *c != '\0'; c++) { -if (*c == '/') -*c = '_'; -} - -filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path, - sanitized_name); -g_free(sanitized_name); +memory = ROUND_UP(memory, pagesize); -fd = mkstemp(filename); +fd = open_ram_file_path(block, path, memory); if (fd < 0) { error_setg_errno(errp, errno, "unable to create backing store for path %s", path); -g_free(filename); goto error; } -unlink(filename); -g_free(filename); - -memory = ROUND_UP(memory, pagesize); - -/* - * ftruncate is not supported by hugetlbfs in older - * hosts, so don't bother bailing out on errors. - * If anything goes wrong with it under other filesystems, - * mmap will fail. - */ -if (ftruncate(fd, memory)) { -perror("ftruncate"); -} area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); if (area == MAP_FAILED) { -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html