Re: [Qemu-devel] Re: [kvm-devel] [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)
And I don't understand why, when going along with the bundle idea, you are suddenly creating a new configuration file format of your own. There is a standard format, XML or text based, which can be easily parsed with OSX APIs (and thus likely *Step APIs as well) as a dictionary, allowing structured information to be stored and thus allowing Qemu backends to store their own settings alongside. See attached one my bundles' configuration.plist file (XML serialized) - relevant is only the Arguments entry, everything else is GUI specific. I'm not saying this format (the structure) were perfect but the underlying property list format (key, value-type) is standard for such bundles, so instead of re-inventing the wheel please take a look at how it's being done! I would like to think that having command line options separated by spaces or newlines is a new configuration file format, but it's not. This patch is not meant as a replacement for libvirt or any other backend. It's just a replacement for shell scripts that serve the only purpose of storing command line options. It should be a simple solution for a simple problem. The complex solution for the complex problem is already there and is called libvirt. I don't want to have QEMU parse serialized XML just to replace my '-m 512 -soundhw es1370 -net nic,model=rtl8139' command line. I would simply like to store that command line somewhere. Anyways, it's obvious that this is a delicate issue for many people here. I think the most non-disruptive way of doing this is the '-c' command line option. It doesn't change QEMU default behaviour, it doesn't add new hyphen-less options. I insist on the fact that this should be a simple solution for a simple problem. Cheers, Jorge
Re: [Qemu-devel] Re: [kvm-devel] [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)
On 9/1/07, Andreas Färber [EMAIL PROTECTED] wrote: What you are basically doing is taking up the concept of a bundle but call it directory, do not give it a mandatory folder name extension and limit the usefulness of the configuration file to your personal needs. I think the problem here is that the scope of this change is not clear. I _really_ wish to keep this simple. I _really_ wish to avoid having giant command lines and useless shell scripts. I don't mean to reinvent the concept of a bundle. Using directories like that seemed a good way of achieving the purpose of _not_ having to deal with command lines. Now, it seems that a config file switch is a better choice. I hardly think that being able to store command line options in a plain text file is just a matter of _my_ personal needs. The configuration file format you are proposing is new because you are proposing it now while, as one example, Q has previously introduced the concept of bundling a Qemu machine on the Mac. And the .plist format has existed for bundles even long before that. Think about it: If you force frontends to use their own configuration files inside the bundle because you want to keep yours simple then you force frontends to parse two different configuration files. Whereas you yourself just said parsing one XML file was already too much for you! Standardizing a more advanced configuration file format here and now would enable frontends to exchange such bundles, retaining their information. By saying you just want a replacement for your command line scripts you are ignoring that other people and projects may have more advanced needs. Those people that have more advanced needs can use the frontends. This is not meant (necessarily) for frontends. It's meant exactly to replace command line scripts. Oh and this has nothing to do with any virtualization libraries, virtualization is not what I (or Q) do so that is no solution at all. This is all about invoking QEMU. libvirt can be perfectly used to invoke QEMU. The name might be ambiguous, but the functionality is not. In fact, using or not using KVM with QEMU reduces to selecting a checkbox in libvirt. It's just another frontend. So in the end it simply means that you are taking an existing concept and apply it half-heartedly and short-sighted: I might be wrong but it seems you have a *nix viewpoint, are not used to working with bundles and therefore re-inventing them differently. There are in fact real bundles on many Linux systems, have a look at pcsc-lite, e.g. /usr/lib/pcsc/drivers/ifd-ccid.bundle. This driver bundle has the default extension of .bundle, obviously not being opened as a document by a user, but users do start up virtual machines with QEMU so a custom extension is useful and necessary to detect that the directory represents in fact a bundle and more specifically a QEMU machine bundle. This is probably true. I never intended to implement the full concept of a bundle. Again, I was looking for a way to avoid writing gigantic command lines; a way that would have consensus in the QEMU community. As I said in my last mail, the config file switch seems more suited to do this. For what I intended to solve, implementing the whole bundle thing is overkill. I'd rather not move the complexity of the bundle into QEMU, but rather leave it on the frontends. So, let's just have a simple way of storing command line options in a config file; a way that does not conflict with existing frontends. Cheers, Jorge
[Qemu-devel] [PATCH][RFC] Allowing QEMU to directly execute a directory (and storing command line options in it)
Hi all,
The last time this issue was discussed, the executable-directory idea
gained more consensus than the snapshot-based idea. This patch
implements my perception of the first idea. Non-Windows for now, as I
can't test on a Windows system. Suggestions and constructive criticism
more than welcome.
Cheers,
Jorge
This patch allows QEMU to execute a directory with a special format.
This patch allows storing command line options in a configuration file inside
a directory and then directly executing that directory. A simple check is
included to prevent the configuration file to access image files outside
the executed directory. Extra command line options can be passed on invocation,
which will take precedence over the ones stored in the configuration file.
The configuration file specifies, on its first line, the image file to use.
The rest of the file specifies command line options separated by spaces or
newlines. Careful reconstruction of the command line makes sure the speficied
image file gets executed even if other image files were included later in the
configuration file.
Signed-off-by: Jorge Lucángeli Obes
---
diff --git a/qemu/vl.c b/qemu/vl.c
index fcc899b..88cefd2 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -42,8 +42,8 @@
#include netinet/in.h
#include dirent.h
#include netdb.h
-#ifdef _BSD
#include sys/stat.h
+#ifdef _BSD
#ifndef __APPLE__
#include libutil.h
#endif
@@ -6367,9 +6367,16 @@ int main_loop(void)
void help(void)
{
printf(QEMU PC emulator version QEMU_VERSION , Copyright (c)
2003-2007 Fabrice Bellard\n
- usage: %s [options] [disk_image]\n
+ usage: %s [options] [disk_image|folder]\n
\n
+#ifdef _WIN32
'disk_image' is a raw hard image image for IDE hard disk 0\n
+#else
+ 'disk_image' is a raw hard image image for IDE hard disk 0 or\n
+ 'folder' is a folder with a file 'config' containing in
the first line\n
+ the name of an image file inside the folder and in the
rest of the file\n
+ options separated by ' ' or '\\n'\n
+#endif
\n
Standard options:\n
-M machine select emulated machine (-M ? for list)\n
@@ -6892,6 +6899,20 @@ void qemu_get_launch_info(int *argc, char
***argv, int *opt_daemonize, const cha
*opt_incoming = incoming;
}
+char *dir_file_cat(const char *folder, const char *file) {
+int foldlen = strlen(folder);
+int filelen = strlen(file);
+int reslen = foldlen + 1 + filelen + 1;
+
+char *res = malloc(sizeof(char) * reslen);
+
+pstrcpy(res, reslen, folder);
+strncat(res, /, 1);
+strncat(res, file, filelen);
+
+return res;
+}
+
int main(int argc, char **argv)
{
#ifdef CONFIG_GDBSTUB
@@ -7003,7 +7024,120 @@ int main(int argc, char **argv)
nb_nics = 0;
/* default mac address of the first network interface */
+
+#ifndef _WIN32
+#define DIR_CMDLINE_SIZE 113
+int hd_found = 0;
+char *dir, *opts;
+struct stat *s = NULL;
+
+optind = 1;
+for(;;) {
+if (optind = argc)
+break;
+
+dir = argv[optind++];
+
+if (dir[0] != '-') {
+hd_found = 1;
+break;
+}
+}
+if (hd_found) {
+s = malloc(sizeof(*s));
+
+if (stat(dir, s) 0) {
+/* Error */
+fprintf(stderr, unable to stat: '%s'\n,
+dir);
+exit(1);
+}
+
+if (S_ISDIR(s-st_mode)) {
+/* The user specified a directory, search for ./config */
+int configlen = strlen(dir);
+configlen += 8; /* /config\0 */
+char *config = malloc(sizeof(char) * configlen);
+
+pstrcpy(config, configlen, dir);
+strncat(config, /config, 7);
+
+int fd_config;
+
+if ((fd_config = open(config, 0)) 0) {
+/* Error */
+if (errno == ENOENT)
+fprintf(stderr, config file not found: '%s'\n,
+config);
+else
+fprintf(stderr, unable to open config file: '%s'\n,
+config);
+exit(1);
+}
+
+opts = malloc(sizeof(char) * (DIR_CMDLINE_SIZE));
+
+ssize_t readb = read(fd_config, opts, (DIR_CMDLINE_SIZE) - 1);
+
+opts[readb] = '\0';
+
+char *filename = strsep(opts, \n);
+
+if (filename == NULL) {
+/* Error */
+fprintf(stderr, malformed configuration file: '%s'\n,
+config);
+exit(1);
+} else if (strchr(filename, '/') != NULL) {
+/* Error */
+fprintf(stderr, '%s' may point outside folder '%s'\n
+avoid using '/' in config file\n,
+filename, dir);
+exit(1);
+}
+
+char tmpopts[DIR_CMDLINE_SIZE
[Qemu-devel] Making qemu images executable (and store command line arguments in them =P)
I've been giving some thought to Anthony's idea: http://kvm.qumranet.com/kvmwiki/Specs/StoringCommandLineInImage However, maybe I'm just too much on vacations, but I don't seem to come up with a nice way of doing this. Everything keeps coming back to creating a new 'container' image format and then implementing block layer functions that only add the number of sectors occupied by the command-line to the read and write calls made by QEMU, and then just relay those calls to the image-specific functions. That doesn't sound very efficient. The '#!' trick works nice with scripts, but I don't see it playing very well with images. ¿Comments? ¿Pointers? Cheers, Jorge
Re: [kvm-devel] [Qemu-devel] Re: Storing command line options in images
On 8/14/07, Anthony Liguori [EMAIL PROTECTED] wrote: On Mon, 2007-08-13 at 20:39 +0100, Thiemo Seufer wrote: Jorge Lucángeli Obes wrote: [snip] When I read Avi's TODO, I basically thought about getting rid of the long command lines I had to store in scripts. I wanted to write that command line once, and then forgetting about it, until I needed to change it. Instead of inventing great and wonderfully complicated schemes, the most sensible way I can think of is to recycle a feature which is now implemented in the GNU toolchain, and apparently stems from Windows: qemu @qemu.cfg where qemu.cfg is a file which contains the command line arguments. (This is also low-maintenance, as it allows to re-use the existing parser. No need for duplicated logic.) In this case, it's also just as easy to make a shell script. I think an important goal here is to automatically associate the options for a VM with the actual disk image. It's not just about storing said options in a file. On my 64-bit Xubuntu Feisty I get: qemu: could not open hard disk image '@config' Anyways, as Anthony said, we would like to have only one file describing the VM. Since we already have the disk image, we were looking for ways to reuse that image for this purpose. I did not know about '@config', and if I can get it to work, I like it better than a shell script. However, I think it does not completely solve the problem. It still means two files per VM. Cheers, Jorge
Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images
On 8/11/07, Anthony Liguori [EMAIL PROTECTED] wrote: On Sat, 2007-08-11 at 22:28 +0100, Philip Boulain wrote: This works, so long as qemu disregards -read-args-from-image unless it's being called as an interpreter. Otherwise, you've put the lock and the key in the same place. :) Yes. This is exactly what I think the behavior ought to be. Ack. (Side thought: presumably, we're assuming that the in-file and on-command-line arguments are unioned, ideally with the latter taking precidence if mutually exclusive.) Yup, I was thinking the same thing too. Once the KVM wiki comes back online I'll write something up on a wiki page. Great, that was also my idea. Anyone interested in taking a stab at this one? I will try and start getting it done this week. Cheers, Jorge
[Qemu-devel] Storing command line options in images
Hi all, From what I've gathered, it seems that we have basically four options at hand. I think it's important to notice, however, that whatever comes out of this will probably be, as Avi said, a low-end solution. IMHO there's room to have both the high-end libvirt-like approach, and the shell replacer approach. So let's see: 1- We could go the way of the patches I've posted, adding the comments everyone's made. 1a- It's a good idea to actively signal QEMU to read command line options from the image file, and not have it on by default. 1b- As Avi said, there are many guest-related options that would be good to store _somewhere_. The user always has the option of using qemu-img to review the options that the VM is going to use. I think that having a valid set of options that the user can store will complicate things too much. 2- We could bump qcow's version number and add command line options as first-class citizens of the image world. 2a- As Anthony suggested, it could be a good starting point to make sure these version transition happen in a smoother way. 3- We could add command line options as plain text in the image itself. Sounds (to me) risky and not very user friendly. 4- We could have configuration files associated with the host and guests. 4a- Embedded XML. 4b- Separate files. What do you guys think? I'm partial to 1 or 2. Since the beginning my approach was that of a simple solution for a simple feature. Cheers, Jorge
[Qemu-devel] [PATCH 1/4][RFC] Allow storing arbitrary annotations in qcow2 images
This patch adds an extra 'annot' field to qcow2 snapshots, and updates
serialization functions.
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/block-qcow2.c b/qemu/block-qcow2.c
index 0f7a069..361d300 100644
--- a/qemu/block-qcow2.c
+++ b/qemu/block-qcow2.c
@@ -106,6 +106,8 @@ typedef struct QCowSnapshot {
uint32_t l1_size;
char *id_str;
char *name;
+char *annot;
+
uint32_t vm_state_size;
uint32_t date_sec;
uint32_t date_nsec;
@@ -1370,6 +1372,7 @@ static void qcow_free_snapshots(BlockDriverState *bs)
for(i = 0; i s-nb_snapshots; i++) {
qemu_free(s-snapshots[i].name);
qemu_free(s-snapshots[i].id_str);
+qemu_free(s-snapshots[i].annot);
}
qemu_free(s-snapshots);
s-snapshots = NULL;
@@ -1401,12 +1404,18 @@ static int qcow_read_snapshots(BlockDriverState *bs)
sn-date_sec = be32_to_cpu(h.date_sec);
sn-date_nsec = be32_to_cpu(h.date_nsec);
sn-vm_clock_nsec = be64_to_cpu(h.vm_clock_nsec);
-extra_data_size = be32_to_cpu(h.extra_data_size);
+extra_data_size = be32_to_cpu(h.extra_data_size);
id_str_size = be16_to_cpu(h.id_str_size);
name_size = be16_to_cpu(h.name_size);
+sn-annot = qemu_malloc(extra_data_size + 1);
+if (!sn-annot)
+goto fail;
+if (bdrv_pread(s-hd, offset, sn-annot, extra_data_size) !=
extra_data_size)
+goto fail;
offset += extra_data_size;
+sn-annot[extra_data_size] = '\0';
sn-id_str = qemu_malloc(id_str_size + 1);
if (!sn-id_str)
@@ -1439,7 +1448,7 @@ static int qcow_write_snapshots(BlockDriverState *bs)
QCowSnapshotHeader h;
int i, name_size, id_str_size, snapshots_size;
uint64_t data64;
-uint32_t data32;
+uint32_t data32, extra_data_size;
int64_t offset, snapshots_offset;
/* compute the size of the snapshots */
@@ -1448,6 +1457,7 @@ static int qcow_write_snapshots(BlockDriverState *bs)
sn = s-snapshots + i;
offset = align_offset(offset, 8);
offset += sizeof(h);
+offset += strlen(sn-annot);
offset += strlen(sn-id_str);
offset += strlen(sn-name);
}
@@ -1455,7 +1465,7 @@ static int qcow_write_snapshots(BlockDriverState *bs)
snapshots_offset = alloc_clusters(bs, snapshots_size);
offset = snapshots_offset;
-
+
for(i = 0; i s-nb_snapshots; i++) {
sn = s-snapshots + i;
memset(h, 0, sizeof(h));
@@ -1465,7 +1475,9 @@ static int qcow_write_snapshots(BlockDriverState *bs)
h.date_sec = cpu_to_be32(sn-date_sec);
h.date_nsec = cpu_to_be32(sn-date_nsec);
h.vm_clock_nsec = cpu_to_be64(sn-vm_clock_nsec);
-
+
+extra_data_size = strlen(sn-annot);
+h.extra_data_size = cpu_to_be32(extra_data_size);
id_str_size = strlen(sn-id_str);
name_size = strlen(sn-name);
h.id_str_size = cpu_to_be16(id_str_size);
@@ -1474,6 +1486,9 @@ static int qcow_write_snapshots(BlockDriverState *bs)
if (bdrv_pwrite(s-hd, offset, h, sizeof(h)) != sizeof(h))
goto fail;
offset += sizeof(h);
+if (bdrv_pwrite(s-hd, offset, sn-annot, extra_data_size) !=
extra_data_size)
+goto fail;
+offset += extra_data_size;
if (bdrv_pwrite(s-hd, offset, sn-id_str, id_str_size) != id_str_size)
goto fail;
offset += id_str_size;
@@ -1570,6 +1585,10 @@ static int qcow_snapshot_create(BlockDriverState *bs,
sn-name = qemu_strdup(sn_info-name);
if (!sn-name)
goto fail;
+sn-annot = qemu_strdup(sn_info-annot);
+if (!sn-annot)
+goto fail;
+
sn-vm_state_size = sn_info-vm_state_size;
sn-date_sec = sn_info-date_sec;
sn-date_nsec = sn_info-date_nsec;
@@ -1680,6 +1699,8 @@ static int qcow_snapshot_delete(BlockDriverState
*bs, const char *snapshot_id)
qemu_free(sn-id_str);
qemu_free(sn-name);
+qemu_free(sn-annot);
+
memmove(sn, sn + 1, (s-nb_snapshots - snapshot_index - 1) * sizeof(*sn));
s-nb_snapshots--;
ret = qcow_write_snapshots(bs);
@@ -1707,10 +1728,14 @@ static int qcow_snapshot_list(BlockDriverState *bs,
for(i = 0; i s-nb_snapshots; i++) {
sn_info = sn_tab + i;
sn = s-snapshots + i;
+
pstrcpy(sn_info-id_str, sizeof(sn_info-id_str),
sn-id_str);
pstrcpy(sn_info-name, sizeof(sn_info-name),
sn-name);
+pstrcpy(sn_info-annot, sizeof(sn_info-annot),
+sn-annot);
+
sn_info-vm_state_size = sn-vm_state_size;
sn_info-date_sec = sn-date_sec;
sn_info-date_nsec = sn-date_nsec;
diff --git a/qemu/vl.h b/qemu/vl.h
index 43f56bd..f0273fd 100644
--- a/qemu/vl.h
+++ b/qemu/vl.h
@@ -589,6 +589,7 @@ typedef struct QEMUSnapshotInfo {
/* the following fields are informative. They are not needed for
the consistency of the snapshot */
char
[Qemu-devel] [PATCH 2/4][RFC] Add functions to manipulate image annotations
This patch adds block driver functions to use qcow2 image annotations.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/block.c b/qemu/block.c
index 39ec37a..4d794f6 100644
--- a/qemu/block.c
+++ b/qemu/block.c
@@ -56,6 +56,8 @@ static int bdrv_write_em(BlockDriverState *bs,
int64_t sector_num,
static BlockDriverState *bdrv_first;
static BlockDriver *first_drv;
+static char *filter_name(char *name);
+
int path_is_absolute(const char *path)
{
const char *p;
@@ -1037,6 +1039,8 @@ char *bdrv_snapshot_dump(char *buf, int
buf_size, QEMUSnapshotInfo *sn)
snprintf(buf, buf_size,
%-10s%-20s%7s%20s%15s,
ID, TAG, VM SIZE, DATE, VM CLOCK);
+} else if (bdrv_snapshot_annotated(sn)) {
+snprintf(buf, buf_size, %s: '%s', filter_name(sn-name), sn-annot);
} else {
ti = sn-date_sec;
#ifdef _WIN32
@@ -1361,3 +1365,128 @@ void bdrv_set_locked(BlockDriverState *bs, int locked)
drv-bdrv_set_locked(bs, locked);
}
}
+
+/**/
+/* image annotation support */
+
+static char *prepare_name(const char *name)
+{
+char *res = malloc(1024 * sizeof(char));
+
+res[0] = '\n';
+return strncat(res, name, 1022);
+}
+
+static char *filter_name(char *name)
+{
+return strtok(name, \n);
+}
+
+int bdrv_snapshot_annotated(QEMUSnapshotInfo *sn)
+{
+ return (sn-name[0] == '\n'
+ sn-vm_state_size == 0
+ *sn-annot != '\0');
+}
+
+int bdrv_set_annot(BlockDriverState *bs,
+ const char *name,
+ const char *annotation)
+{
+int nbsnaps, i;
+int ret = 0;
+
+char *fltname;
+char *prepname;
+char *id_annot = NULL;
+
+QEMUSnapshotInfo *sn_info;
+BlockDriver *drv = bs-drv;
+
+nbsnaps = drv-bdrv_snapshot_list(bs, sn_info);
+
+if (nbsnaps 0) {
+ret = -1;
+goto end;
+}
+
+for (i = 0; i nbsnaps; i++) {
+fltname = filter_name(sn_info[i].name);
+
+if (sn_info[i].name[0] == '\n'
+!strcmp(fltname, name)
+sn_info[i].vm_state_size == 0) {
+
+id_annot = sn_info[i].id_str;
+break;
+}
+}
+
+if (id_annot != NULL)
+drv-bdrv_snapshot_delete(bs, id_annot);
+
+if (strlen(annotation) == 0)
+goto end;
+
+QEMUSnapshotInfo *new_sn_info = qemu_malloc(sizeof(QEMUSnapshotInfo));
+
+if (!new_sn_info) {
+ret = -ENOMEM;
+goto end;
+}
+
+prepname = prepare_name(name);
+
+// No id, set by create
+new_sn_info-id_str[0] = '\0';
+pstrcpy(new_sn_info-name, 1024, prepname);
+pstrcpy(new_sn_info-annot, 1024, annotation);
+
+new_sn_info-vm_state_size = new_sn_info-date_sec =
+new_sn_info-date_nsec = new_sn_info-vm_clock_nsec = 0;
+
+if (drv-bdrv_snapshot_create(bs, new_sn_info) 0)
+ret = -1;
+
+free(prepname);
+free(new_sn_info);
+
+end:
+free(sn_info);
+return ret;
+}
+
+char *bdrv_get_annot(BlockDriverState *bs, const char *name)
+{
+int nbsnaps, i;
+char *filteredn;
+char *res;
+
+QEMUSnapshotInfo *sn_info = NULL;
+
+res = malloc(1024 * sizeof(char));
+if (!res)
+return NULL;
+
+nbsnaps = bs-drv-bdrv_snapshot_list(bs, sn_info);
+
+if (nbsnaps 0) {
+free(sn_info);
+free(res);
+return NULL;
+}
+
+for (i = 0; i nbsnaps; i++) {
+filteredn = filter_name(sn_info[i].name);
+
+if (sn_info[i].name[0] == '\n'
+!strcmp(filteredn, name)
+sn_info[i].vm_state_size == 0) {
+
+pstrcpy(res, 1024, sn_info[i].annot);
+break;
+}
+}
+
+return res;
+}
This patch adds block driver functions to use qcow2 image annotations.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/block.c b/qemu/block.c
index 39ec37a..4d794f6 100644
--- a/qemu/block.c
+++ b/qemu/block.c
@@ -56,6 +56,8 @@ static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
static BlockDriverState *bdrv_first;
static BlockDriver *first_drv;
+static char *filter_name(char *name);
+
int path_is_absolute(const char *path)
{
const char *p;
@@ -1037,6 +1039,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
snprintf(buf, buf_size,
%-10s%-20s%7s%20s%15s,
ID, TAG, VM SIZE, DATE, VM CLOCK);
+} else if (bdrv_snapshot_annotated(sn)) {
+snprintf(buf, buf_size, %s: '%s', filter_name(sn-name), sn-annot);
} else {
ti = sn-date_sec;
#ifdef _WIN32
@@ -1361,3 +1365,128 @@ void bdrv_set_locked(BlockDriverState *bs, int locked)
drv-bdrv_set_locked(bs, locked);
}
}
+
+/**/
+/* image annotation
[Qemu-devel] [PATCH 3/4][RFC] Add qemu-img option to store command line options into qcow2 images
This patch adds a new qemu-img option to store
command line arguments in qcow2 snapshots.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/qemu-img.c b/qemu/qemu-img.c
index a259546..afa1fcc 100644
--- a/qemu/qemu-img.c
+++ b/qemu/qemu-img.c
@@ -98,6 +98,7 @@ void help(void)
QEMU disk image utility\n
\n
Command syntax:\n
+ cmdline filename \command_line\ (qcow2 format only)\n
create [-e] [-b base_image] [-f fmt] filename [size]\n
commit [-f fmt] filename\n
convert [-c] [-e] [-f fmt] filename [-O output_fmt]
output_filename\n
@@ -105,6 +106,8 @@ void help(void)
\n
Command parameters:\n
'filename' is a disk image filename\n
+ 'command_line' is a list of command line options to be
stored in the image,\n
+ an empty string clears the stored command line options\n
'base_image' is the read-only disk image which is used
as base for a copy on\n
write image; the copy on write image only stores the
modified data\n
'fmt' is the disk image format. It is guessed
automatically in most cases\n
@@ -317,6 +320,33 @@ static int img_create(int argc, char **argv)
return 0;
}
+static int img_cmdline(int argc, char **argv)
+{
+char *filename;
+char *annotation;
+
+BlockDriverState *bs;
+
+char *aname = commandline_args;
+
+if (argc != 4)
+help();
+
+filename = argv[2];
+annotation = argv[3];
+
+bs = bdrv_new_open(filename, qcow2);
+if (!bs)
+error(Could not open qcow2 image '%s', filename);
+
+if (bdrv_set_annot(bs, aname, (const char *)annotation) 0) {
+error(Could not store command line options into '%s', filename);
+}
+
+bdrv_delete(bs);
+return 0;
+}
+
static int img_commit(int argc, char **argv)
{
int c, ret;
@@ -577,11 +607,19 @@ static void dump_snapshots(BlockDriverState *bs)
nb_sns = bdrv_snapshot_list(bs, sn_tab);
if (nb_sns = 0)
return;
-printf(Snapshot list:\n);
+
+printf(\nAnnotations:\n);
+for (i = 0; i nb_sns; i++) {
+sn = sn_tab[i];
+if (bdrv_snapshot_annotated(sn))
+printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn));
+}
+printf(\nSnapshot list:\n);
printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
for(i = 0; i nb_sns; i++) {
sn = sn_tab[i];
-printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn));
+if (!bdrv_snapshot_annotated(sn))
+printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn));
}
qemu_free(sn_tab);
}
@@ -673,7 +711,9 @@ int main(int argc, char **argv)
help();
cmd = argv[1];
optind++;
-if (!strcmp(cmd, create)) {
+if (!strcmp(cmd, cmdline)) {
+ img_cmdline(argc, argv);
+} else if (!strcmp(cmd, create)) {
img_create(argc, argv);
} else if (!strcmp(cmd, commit)) {
img_commit(argc, argv);
This patch adds a new qemu-img option to store
command line arguments in qcow2 snapshots.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/qemu-img.c b/qemu/qemu-img.c
index a259546..afa1fcc 100644
--- a/qemu/qemu-img.c
+++ b/qemu/qemu-img.c
@@ -98,6 +98,7 @@ void help(void)
QEMU disk image utility\n
\n
Command syntax:\n
+ cmdline filename \command_line\ (qcow2 format only)\n
create [-e] [-b base_image] [-f fmt] filename [size]\n
commit [-f fmt] filename\n
convert [-c] [-e] [-f fmt] filename [-O output_fmt] output_filename\n
@@ -105,6 +106,8 @@ void help(void)
\n
Command parameters:\n
'filename' is a disk image filename\n
+ 'command_line' is a list of command line options to be stored in the image,\n
+ an empty string clears the stored command line options\n
'base_image' is the read-only disk image which is used as base for a copy on\n
write image; the copy on write image only stores the modified data\n
'fmt' is the disk image format. It is guessed automatically in most cases\n
@@ -317,6 +320,33 @@ static int img_create(int argc, char **argv)
return 0;
}
+static int img_cmdline(int argc, char **argv)
+{
+char *filename;
+char *annotation;
+
+BlockDriverState *bs;
+
+char *aname = commandline_args;
+
+if (argc != 4)
+help();
+
+filename = argv[2];
+annotation = argv[3];
+
+bs = bdrv_new_open(filename, qcow2);
+if (!bs)
+error(Could not open qcow2 image '%s', filename);
+
+if (bdrv_set_annot(bs, aname, (const char *)annotation) 0) {
+error(Could not store command line options into '%s', filename);
+}
+
+bdrv_delete(bs
[Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images
This patch makes QEMU check for command line options stored in qcow2 images.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/vl.c b/qemu/vl.c
index 4ad39f1..1d28794 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -184,6 +184,11 @@ int time_drift_fix = 0;
const char *cpu_vendor_string;
/***/
+/* Image annotation support */
+
+char *img_get_annot(char *filename);
+
+/***/
/* x86 ISA bus support */
target_phys_addr_t isa_mem_base = 0;
@@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
char usb_devices[MAX_USB_CMDLINE][128];
int usb_devices_index;
int fds[2];
+char *tmpannot;
+char annot[1024];
+int done = 0;
+unsigned int nbtoks = 0;
+char *tok;
+BlockDriver *drv;
+BlockDriverState *bs;
saved_argc = argc;
saved_argv = argv;
@@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
nb_nics = 0;
/* default mac address of the first network interface */
+bdrv_init();
+
+drv = bdrv_find_format(qcow2);
+
+if (argc 1 argv[1][0] != '-') {
+bs = bdrv_new();
+if (!bs) {
+fprintf(stderr, Not enough memory);
+exit(1);
+}
+if (bdrv_open2(bs, argv[1], 0, drv) 0) {
+fprintf(stderr, Could not open '%s', argv[1]);
+bdrv_delete(bs);
+exit(1);
+}
+
+tmpannot = bdrv_get_annot(bs, commandline_args);
+if (tmpannot) {
+pstrcpy(annot, 1024, tmpannot);
+
+do {
+tok = strtok(nbtoks == 0? tmpannot : NULL, );
+
+if (tok != NULL)
+nbtoks++;
+else
+done = 1;
+} while (!done);
+
+free(tmpannot);
+
+if (nbtoks 0) {
+char **argvprime = malloc((nbtoks + argc) * sizeof(char*));
+
+for (i = 0; i argc; i++)
+argvprime[i] = argv[i];
+
+for (i = 0; i nbtoks; i++)
+argvprime[i + argc] = strtok(i == 0? annot : NULL, );
+
+argv = argvprime;
+argc = argc + nbtoks;
+
+for (i = 0; i nbtoks + 2; i++)
+printf(argv[%d] = %s\n, i, argv[i]);
+
+}
+}
+
+bdrv_delete(bs);
+}
+
optind = 1;
for(;;) {
if (optind = argc)
@@ -7558,7 +7622,6 @@ int main(int argc, char **argv)
#endif
/* we always create the cdrom drive, even if no disk is there */
-bdrv_init();
if (cdrom_index = 0) {
bs_table[cdrom_index] = bdrv_new(cdrom);
bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
@@ -7764,5 +7827,8 @@ int main(int argc, char **argv)
main_loop();
quit_timers();
+
+/* was reassigned above so it needs free()ing */
+free(argv);
return 0;
}
This patch makes QEMU check for command line options stored in qcow2 images.
Signed-off-by: Laurent Vivier [EMAIL PROTECTED]
Signed-off-by: Jorge Lucángeli Obes [EMAIL PROTECTED]
---
diff --git a/qemu/vl.c b/qemu/vl.c
index 4ad39f1..1d28794 100644
--- a/qemu/vl.c
+++ b/qemu/vl.c
@@ -184,6 +184,11 @@ int time_drift_fix = 0;
const char *cpu_vendor_string;
/***/
+/* Image annotation support */
+
+char *img_get_annot(char *filename);
+
+/***/
/* x86 ISA bus support */
target_phys_addr_t isa_mem_base = 0;
@@ -6917,6 +6922,13 @@ int main(int argc, char **argv)
char usb_devices[MAX_USB_CMDLINE][128];
int usb_devices_index;
int fds[2];
+char *tmpannot;
+char annot[1024];
+int done = 0;
+unsigned int nbtoks = 0;
+char *tok;
+BlockDriver *drv;
+BlockDriverState *bs;
saved_argc = argc;
saved_argv = argv;
@@ -7000,6 +7012,58 @@ int main(int argc, char **argv)
nb_nics = 0;
/* default mac address of the first network interface */
+bdrv_init();
+
+drv = bdrv_find_format(qcow2);
+
+if (argc 1 argv[1][0] != '-') {
+bs = bdrv_new();
+if (!bs) {
+fprintf(stderr, Not enough memory);
+exit(1);
+}
+if (bdrv_open2(bs, argv[1], 0, drv) 0) {
+fprintf(stderr, Could not open '%s', argv[1]);
+bdrv_delete(bs);
+exit(1);
+}
+
+tmpannot = bdrv_get_annot(bs, commandline_args);
+if (tmpannot) {
+pstrcpy(annot, 1024, tmpannot);
+
+do {
+tok = strtok(nbtoks == 0? tmpannot : NULL, );
+
+if (tok != NULL)
+nbtoks++;
+else
+done = 1;
+} while (!done);
+
+free(tmpannot);
+
+if (nbtoks 0
[Qemu-devel] Re: [kvm-devel] Storing command line options in qcow2 images
On 7/30/07, Avi Kivity [EMAIL PROTECTED] wrote: Jorge Lucángeli Obes wrote: Hi Avi, hi all, I believe that Laurent (copied) is also interested in this area. Copied him. I've started some (very minor) groundwork for this task. My idea was to add an extra annotation field in qcow2 snapshots. In this way, a snapshot can hold abitrary information; for example, command line arguments. Before going any further, I wanted to validate the general idea with the list. I've copied qemu-devel as well. This can benefit qemu as much as kvm and I see no reasons that this shouldn't be merged into kvm through qemu upstream. Furthermore, I'd very much hate to see qemu and kvm image formats diverge, so I'd like to get the qemu maintainer's approval for any format changes or extensions. Agreed. There's a slight overhead as now all qcow2 snapshots have to store a extra (probably) empty pointer. An alternative approach would be to somehow indicate that some snapshots are descriptive snapshots and others are useful snapshots; however, I felt that this would be more complicated. Will storing an extra pointer break existing qcow2 images? If so, we'd better avoid it. It could be a problem, I agree. Maybe it's a better idea not to touch the on-disk format. That would mean leaving 'struct QCowSnapshotHeader' intact, and using existing fields to store our information. The header includes an extra_data_size field that can do the trick. This would of course need approval from qemu developers. Laurent, do you want the patches for the functionality that's working right now? We can divide the (not very big amount of) work that's left. It consists basically on teaching qemu how to look for command line options, if none are present on the current invocation. I don't mind doing it by myself if you want to keep working on the other stuff you've been posting patches about. Just to be clear, the actual embedding of command line options into the image would be done with qemu-img. It would be qemu's job to validate this when the image is loaded. Another way to do this could be to add an option to qemu that does the embedding. This would have the benefit of free option validation, but I felt that image-related operations should be used from qemu-img. Cheers, Jorge
