[Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Serge E. Hallyn]

Currently, if the user doesn't pass a uuid, the system uuid is set to
all zeros.  This patch generates a random one instead.

Is there a reason to prefer all zeros?  If not, can a patch like this
one be applied?

Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
---
 vl.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 112b0e0..2b53b62 100644
--- a/vl.c
+++ b/vl.c
@@ -247,7 +247,9 @@ int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 uint64_t node_cpumask[MAX_NODES];
 
+#include <uuid/uuid.h>
 uint8_t qemu_uuid[16];
+bool uuid_set = false;
 
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
@@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
                             " Wrong format.\n");
                     exit(1);
                 }
+                uuid_set = true;
                 break;
 	    case QEMU_OPTION_option_rom:
 		if (nb_option_roms >= MAX_OPTION_ROMS) {
@@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
         exit(0);
     }
 
+    if (!uuid_set) {
+        uuid_t uuid;
+        uuid_generate(uuid);
+        for (i = 0; i < 16; i++) {
+            qemu_uuid[i] = uuid[i];
+        }
+    }
+
     /* Open the logfile at this point, if necessary. We can't open the logfile
      * when encountering either of the logging options (-d or -D) because the
      * other one may be encountered later on the command line, changing the
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Andreas Färber]

Am 26.03.2012 17:13, schrieb Serge E. Hallyn:
> Currently, if the user doesn't pass a uuid, the system uuid is set to
> all zeros.  This patch generates a random one instead.
> 
> Is there a reason to prefer all zeros?

Yes, documented somewhere in the archives, we wanted to have
reproducible defaults in QEMU (cf. MAC address, IP addresses) so that it
doesn't change for each invocation or depending on host.

As a general rule, randomization should be done either explicitly (-uuid
`uuidgen` or -generate-me-a-uuid) or via frontends such as libvirt.

If all zeros causes genuine problems then we should change the default,
taking care of backwards compatibility as usual.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Serge E. Hallyn]

Quoting Andreas Färber (afaerber@suse.de):
> Am 26.03.2012 17:13, schrieb Serge E. Hallyn:
> > Currently, if the user doesn't pass a uuid, the system uuid is set to
> > all zeros.  This patch generates a random one instead.
> > 
> > Is there a reason to prefer all zeros?
> 
> Yes, documented somewhere in the archives, we wanted to have
> reproducible defaults in QEMU (cf. MAC address, IP addresses) so that it
> doesn't change for each invocation or depending on host.

Thanks.  Though I don't know of a case offhand, I guess I could imagine
a case where a guest's userspace acts differently (and mis-behaves)
based on the random uuid...

> As a general rule, randomization should be done either explicitly (-uuid
> `uuidgen` or -generate-me-a-uuid) or via frontends such as libvirt.
> 
> If all zeros causes genuine problems then we should change the default,
> taking care of backwards compatibility as usual.

The bug this was in reply to is at http://pad.lv/959308 .  IIUC the main
problem is that our crash database uses the uuid, so users need to set
one to report bugs.  (There was also a suggestion that Microsoft requires
it for Logo Certification.)  My suggestion was also to have callers
specify it, but the crash db issue means we'd have to wrap all calls to
qemu.

So thanks - I understand if this patch doesn't make it upstream.  I'll
just carry a patch in our package in that case.

thanks,
-serge

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Brian Jackson]

On Mon, 26 Mar 2012 10:13:40 -0500, Serge E. Hallyn <serge@hallyn.com>  
wrote:

> Currently, if the user doesn't pass a uuid, the system uuid is set to
> all zeros.  This patch generates a random one instead.
>
> Is there a reason to prefer all zeros?  If not, can a patch like this
> one be applied?
>
> Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
>  vl.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 112b0e0..2b53b62 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -247,7 +247,9 @@ int nb_numa_nodes;
>  uint64_t node_mem[MAX_NODES];
>  uint64_t node_cpumask[MAX_NODES];
> +#include <uuid/uuid.h>



This adds a hard dep on libuuid (prior to this it's optional and only  
impacts the vdi block driver).



>  uint8_t qemu_uuid[16];
> +bool uuid_set = false;
> static QEMUBootSetHandler *boot_set_handler;
>  static void *boot_set_opaque;
> @@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
>                              " Wrong format.\n");
>                      exit(1);
>                  }
> +                uuid_set = true;
>                  break;
>  	    case QEMU_OPTION_option_rom:
>  		if (nb_option_roms >= MAX_OPTION_ROMS) {
> @@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
>          exit(0);
>      }
> +    if (!uuid_set) {
> +        uuid_t uuid;
> +        uuid_generate(uuid);
> +        for (i = 0; i < 16; i++) {
> +            qemu_uuid[i] = uuid[i];
> +        }
> +    }
> +
>      /* Open the logfile at this point, if necessary. We can't open the  
> logfile
>       * when encountering either of the logging options (-d or -D)  
> because the
>       * other one may be encountered later on the command line, changing  
> the

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Serge E. Hallyn]

Quoting Brian Jackson (iggy@theiggy.com):
> On Mon, 26 Mar 2012 10:13:40 -0500, Serge E. Hallyn
> <serge@hallyn.com> wrote:
> 
> >Currently, if the user doesn't pass a uuid, the system uuid is set to
> >all zeros.  This patch generates a random one instead.
> >
> >Is there a reason to prefer all zeros?  If not, can a patch like this
> >one be applied?
> >
> >Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
> >---
> > vl.c |   11 +++++++++++
> > 1 files changed, 11 insertions(+), 0 deletions(-)
> >
> >diff --git a/vl.c b/vl.c
> >index 112b0e0..2b53b62 100644
> >--- a/vl.c
> >+++ b/vl.c
> >@@ -247,7 +247,9 @@ int nb_numa_nodes;
> > uint64_t node_mem[MAX_NODES];
> > uint64_t node_cpumask[MAX_NODES];
> >+#include <uuid/uuid.h>
> 
> 
> 
> This adds a hard dep on libuuid (prior to this it's optional and
> only impacts the vdi block driver).

Yup.  If that's a problem we can re-implement our own, if the
change in default behavior is acceptable.

> > uint8_t qemu_uuid[16];
> >+bool uuid_set = false;
> >static QEMUBootSetHandler *boot_set_handler;
> > static void *boot_set_opaque;
> >@@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
> >                             " Wrong format.\n");
> >                     exit(1);
> >                 }
> >+                uuid_set = true;
> >                 break;
> > 	    case QEMU_OPTION_option_rom:
> > 		if (nb_option_roms >= MAX_OPTION_ROMS) {
> >@@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
> >         exit(0);
> >     }
> >+    if (!uuid_set) {
> >+        uuid_t uuid;
> >+        uuid_generate(uuid);
> >+        for (i = 0; i < 16; i++) {
> >+            qemu_uuid[i] = uuid[i];
> >+        }
> >+    }
> >+
> >     /* Open the logfile at this point, if necessary. We can't
> >open the logfile
> >      * when encountering either of the logging options (-d or -D)
> >because the
> >      * other one may be encountered later on the command line,
> >changing the

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Anthony Liguori]

On 03/26/2012 10:13 AM, Serge E. Hallyn wrote:
> Currently, if the user doesn't pass a uuid, the system uuid is set to
> all zeros.  This patch generates a random one instead.
>
> Is there a reason to prefer all zeros?  If not, can a patch like this
> one be applied?
>
> Signed-off-by: Serge Hallyn<serge.hallyn@canonical.com>

The other hypervisors don't have a concept of a transient guest like QEMU does.

There is no state preserved between invocations of QEMU.

Setting a random UUID doesn't seem like the right answer to me as it would 
potentially break Windows VMs.

Perhaps if the DMI UUID isn't set, you could look at the root filesystem's UUID?

Not all platforms have a notion of platform UUID so as Ubuntu supports more 
architectures, this problem would have to be dealt with eventually.

Regards,

Anthony Liguori

> ---
>   vl.c |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 112b0e0..2b53b62 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -247,7 +247,9 @@ int nb_numa_nodes;
>   uint64_t node_mem[MAX_NODES];
>   uint64_t node_cpumask[MAX_NODES];
>
> +#include<uuid/uuid.h>
>   uint8_t qemu_uuid[16];
> +bool uuid_set = false;
>
>   static QEMUBootSetHandler *boot_set_handler;
>   static void *boot_set_opaque;
> @@ -3030,6 +3032,7 @@ int main(int argc, char **argv, char **envp)
>                               " Wrong format.\n");
>                       exit(1);
>                   }
> +                uuid_set = true;
>                   break;
>   	    case QEMU_OPTION_option_rom:
>   		if (nb_option_roms>= MAX_OPTION_ROMS) {
> @@ -3200,6 +3203,14 @@ int main(int argc, char **argv, char **envp)
>           exit(0);
>       }
>
> +    if (!uuid_set) {
> +        uuid_t uuid;
> +        uuid_generate(uuid);
> +        for (i = 0; i<  16; i++) {
> +            qemu_uuid[i] = uuid[i];
> +        }
> +    }
> +
>       /* Open the logfile at this point, if necessary. We can't open the logfile
>        * when encountering either of the logging options (-d or -D) because the
>        * other one may be encountered later on the command line, changing the

Re: [Qemu-devel] [PATCH 1/1] If user doesn't specify a uuid, generate a random one

[Serge E. Hallyn]

Quoting Anthony Liguori (anthony@codemonkey.ws):
> On 03/26/2012 10:13 AM, Serge E. Hallyn wrote:
> >Currently, if the user doesn't pass a uuid, the system uuid is set to
> >all zeros.  This patch generates a random one instead.
> >
> >Is there a reason to prefer all zeros?  If not, can a patch like this
> >one be applied?
> >
> >Signed-off-by: Serge Hallyn<serge.hallyn@canonical.com>
> 
> The other hypervisors don't have a concept of a transient guest like QEMU does.

True, and I generally don't like arguments of the form "but (some other)
does it like this," but...

> There is no state preserved between invocations of QEMU.

Right, and I really don't know how many users (besides me) run kvm
by hand, as opposed to using libvirt or testdrive.  Perhaps testdrive
should be extended.

> Setting a random UUID doesn't seem like the right answer to me as it
> would potentially break Windows VMs.
> 
> Perhaps if the DMI UUID isn't set, you could look at the root filesystem's UUID?

Interesting idea.  I don't know enough to know at which point in the code
qemu would know that, but I can take a look.

> Not all platforms have a notion of platform UUID so as Ubuntu
> supports more architectures, this problem would have to be dealt
> with eventually.

So it sounds like in this form certainly it's nacked :)

thanks,
-serge