Hi,
below some comments regarding the vmctl disk image creation code.
Index: main.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 main.c
--- main.c 5 Jan 2016 16:25:34 -0000 1.13
+++ main.c 10 Jan 2016 15:30:55 -0000
@@ -421,9 +421,6 @@ ctl_create(struct parse_result *res, int
fprintf(stderr, "missing size argument\n");
ctl_usage(res->ctl);
}
- res->size /= 1024 / 1024;
- if (res->size < 1)
- errx(1, "specified image size too small");
ret = create_imagefile(paths[0], res->size);
if (ret != 0) {
errno = ret;
The division is a no-op: 1024 / 1024 is evaluated first, resulting in
res->size /= 1. At that point res->size has already been converted to
megabytes by parse_size().
The condition (res->size < 1) can never be true, because in that case
parse_size() would have returned non-zero, triggering the
errx(1, "invalid size: %s", optarg) in ctl_create().
Index: vmctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 vmctl.c
--- vmctl.c 14 Dec 2015 06:59:07 -0000 1.10
+++ vmctl.c 10 Jan 2016 15:30:55 -0000
@@ -376,7 +376,7 @@ vm_console(struct vmop_info_result *list
*
* Parameters:
* imgfile_path: path to the image file to create
- * imgsize : size of the image file to create
+ * imgsize : size of the image file to create (in MB)
*
* Return:
* EEXIST: The requested image file already exists
@@ -387,8 +387,6 @@ int
create_imagefile(const char *imgfile_path, long imgsize)
{
int fd, ret;
- off_t ofs;
- char ch = '\0';
/* Refuse to overwrite an existing image */
fd = open(imgfile_path, O_RDWR | O_CREAT | O_TRUNC | O_EXCL,
@@ -396,19 +394,11 @@ create_imagefile(const char *imgfile_pat
if (fd == -1)
return (errno);
- ofs = (imgsize * 1024 * 1024) - 1;
-
- /* Set fd pos at desired size */
- if (lseek(fd, ofs, SEEK_SET) == -1) {
- ret = errno;
- close(fd);
- return (ret);
- }
-
- /* Write one byte to fill out the extent */
- if (write(fd, &ch, 1) == -1) {
+ /* Extend to desired size */
+ if (ftruncate(fd, (off_t)imgsize * 1024 * 1024) == -1) {
ret = errno;
close(fd);
+ unlink(imgfile_path);
return (ret);
}
lseek() + write() can be replaced by a slightly shorter ftruncate()
call. Note that using ftruncate() to extend a file is not portable
(Posix allows either zero-filling until the given size is reached, or
alternatively erroring out.), but that shouldn't be a proble as vmm
isn't cross-platform either.
I think it would be nice to unlink() the image file when extending it
fails for consistency with the other error case (the file can't be
created).
cheers,
natano
