Re: [libvirt] [PATCH] qemu: Allow the user to specify vendor and product for disk

[Martin Kletzander]

On 11/08/2012 02:52 AM, Dave Allan wrote:
 On Thu, Nov 08, 2012 at 09:28:06AM +0800, Osier Yang wrote:
 On 2012年11月08日 05:04, Dave Allan wrote:
 On Wed, Nov 07, 2012 at 11:24:43PM +0800, Osier Yang wrote:
 On 2012年11月05日 21:34, Martin Kletzander wrote:
 On 11/05/2012 08:04 AM, Osier Yang wrote:
 QEMU supports to set vendor and product strings for disk since
 1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
 exposes it with new XML elementsvendor   andproduct   of disk
 device.
 ---
  docs/formatdomain.html.in  |   10 +
  docs/schemas/domaincommon.rng  |   10 +
  src/conf/domain_conf.c |   30 
 
  src/conf/domain_conf.h |2 +
  src/qemu/qemu_command.c|   29 
 
  .../qemuxml2argv-disk-scsi-disk-vpd.args   |   13 +++
  .../qemuxml2argv-disk-scsi-disk-vpd.xml|   36 
 
  tests/qemuxml2argvtest.c   |4 ++
  8 files changed, 134 insertions(+), 0 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index c8da33d..cc9e871 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -1657,6 +1657,16 @@
  of 16 hexadecimal digits.
  span class='since'Since 0.10.1/span
/dd
 +dtcodevendor/code/dt
 +ddIf present, this element specifies the vendor of a virtual hard
 +disk or CD-ROM device. It's a not more than 8 bytes 
 alphanumeric string.

 Last sentence doesn't make sense, I suggest changing it to either: It
 can't be longer than 8 alphanumeric characters. or simply Maximum 8
 alphanumeric characters.

 Okay, honestly, I wasn't comfortable with the sentence, but can't
 get a better one at that time, :-) I will change your advise a bit:

 It must not be longer than 8 alphanumeric characters.

 Not to be pedantic, but what happens if you hand it doublebyte
 characters?

 Ah, good question, though QEMU will truncate the string to 8 bytes
 anyway, should we do some translation in libvirt? the problem is
 how to map the doublebytes vendors/product to single bytes ones,
 is there some public specification available? /me starts to think
 if it's snake leg (or breaking fly on the wheel) to do the checking
 if it's too heavy.
 
 Mostly I was curious about how the code handled doublebyte characters,
 but now that I think about it, the SCSI spec mandates 8 bytes of
 ASCII[1], but it sounds like qemu is already enforcing that, so I
 think it's fine just to let it be freeform and note the spec's
 requirement in the documentation.
 

I'd say if QEMU starts (and truncates or whatever) with invalid vendor
name, there's no problem for us to leave the responsibility on the user,
but OTOH when QEMU won't like what it's doing and will eventually
fix/change that, our machines may not start.  So if we know the
limitation (and it is very easy one), why don't we just check (and
mention it in the docs) that we accept maximum 8 (16) alphanumeric
characters, checking just [A-Za-z0-9] instead of doing some 'isalnum()'
magic.  Check is easy, QEMU will be always satisfied with the option, I
see it as a win-win.


Martin

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] qemu: graphics support for simultaneous one of each sdl, vnc, spice

[Alon Levy]

---
 src/qemu/qemu_command.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f9e4d4d..fcdf60c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6186,9 +6186,26 @@ qemuBuildCommandLine(virConnectPtr conn,
 }
 
 if (def-ngraphics  1) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   %s, _(only 1 graphics device is supported));
-goto error;
+int sdl = 0, vnc = 0, spice = 0;
+for (i = 0 ; i  def-ngraphics ; ++i) {
+switch (def-graphics[i]-type) {
+case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+++sdl;
+break;
+case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+++vnc;
+break;
+case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+++spice;
+break;
+}
+}
+if (sdl  1 || vnc  1 || spice  1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(only 1 graphics device of each type 
+   (sdl, vnc, spice) is supported));
+goto error;
+}
 }
 
 for (i = 0 ; i  def-ngraphics ; ++i) {
-- 
1.8.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[Peter Krempa]

On 11/08/12 04:29, li guang wrote:

Hi, Jiri, Daniel
Any comment on this patch?



Please be patient. They both attend the KVM Forum conference so they 
won't be around until next week.


Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Allow migration to be cancelled at any phase

[Michal Privoznik]

On 07.11.2012 23:23, Jiri Denemark wrote:
 On Wed, Nov 07, 2012 at 12:03:39 +0100, Michal Privoznik wrote:
 Currently, if user calls virDomainAbortJob we just issue
 'migrate_cancel' and hope for the best. However, if user calls
 the API in wrong phase when migration hasn't been started yet
 (perform phase) the cancel request is just ignored. With this
 patch, the request is remembered and as soon as perform phase
 starts, migration is cancelled.
 ---
  src/qemu/qemu_domain.c|   26 ++
  src/qemu/qemu_domain.h|4 
  src/qemu/qemu_driver.c|   31 +++
  src/qemu/qemu_migration.c |   43 +--
  4 files changed, 98 insertions(+), 6 deletions(-)

 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index a5592b9..031be5f 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -160,6 +160,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
  job-mask = DEFAULT_JOB_MASK;
  job-start = 0;
  job-dump_memory_only = false;
 +job-asyncAbort = false;
  memset(job-info, 0, sizeof(job-info));
  }
  
 @@ -959,6 +960,31 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver, 
 virDomainObjPtr obj)
  return virObjectUnref(obj);
  }
  
 +void
 +qemuDomainObjAbortAsyncJob(virDomainObjPtr obj)
 +{
 +qemuDomainObjPrivatePtr priv = obj-privateData;
 +
 +VIR_DEBUG(Requesting abort of async job: %s,
 +  qemuDomainAsyncJobTypeToString(priv-job.asyncJob));
 +
 +priv-job.asyncAbort = true;
 +}
 +
 +/**
 + * qemuDomainObjAbortAsyncJobRequested:
 + * @obj: domain object
 + *
 + * Was abort requested? @obj MUST be locked when calling this.
 + */
 +bool
 +qemuDomainObjAbortAsyncJobRequested(virDomainObjPtr obj)
 +{
 +qemuDomainObjPrivatePtr priv = obj-privateData;
 +
 +return priv-job.asyncAbort;
 +}
 +
 
 I don't think we need this qemuDomainObjAbortAsyncJobRequested function. It's
 much easier and shorter if the caller just checks priv-job.asyncAbort
 directly. The current code uses functions only for changing the values or more
 complicated reads.
 
  static int
  qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
bool driver_locked,
 diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
 index 9c2f67c..9a31bbe 100644
 --- a/src/qemu/qemu_domain.h
 +++ b/src/qemu/qemu_domain.h
 @@ -111,6 +111,7 @@ struct qemuDomainJobObj {
  unsigned long long start;   /* When the async job started */
  bool dump_memory_only;  /* use dump-guest-memory to do dump 
 */
  virDomainJobInfo info;  /* Async job progress data */
 +bool asyncAbort;/* abort of async job requested */
  };
  
  typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
 @@ -204,6 +205,9 @@ bool qemuDomainObjEndJob(struct qemud_driver *driver,
  bool qemuDomainObjEndAsyncJob(struct qemud_driver *driver,
virDomainObjPtr obj)
  ATTRIBUTE_RETURN_CHECK;
 +void qemuDomainObjAbortAsyncJob(virDomainObjPtr obj);
 +bool qemuDomainObjAbortAsyncJobRequested(virDomainObjPtr obj);
 +
  void qemuDomainObjSetJobPhase(struct qemud_driver *driver,
virDomainObjPtr obj,
int phase);
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 7b8eec6..009c2c8 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -10331,6 +10331,8 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
  virDomainObjPtr vm;
  int ret = -1;
  qemuDomainObjPrivatePtr priv;
 +/* Poll every 50ms for job termination */
 +struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
  
  qemuDriverLock(driver);
  vm = virDomainFindByUUID(driver-domains, dom-uuid);
 @@ -10365,10 +10367,31 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
  goto endjob;
  }
  
 -VIR_DEBUG(Cancelling job at client request);
 -qemuDomainObjEnterMonitor(driver, vm);
 -ret = qemuMonitorMigrateCancel(priv-mon);
 -qemuDomainObjExitMonitor(driver, vm);
 +qemuDomainObjAbortAsyncJob(vm);
 +VIR_DEBUG(Waiting for async job '%s' to finish,
 +  qemuDomainAsyncJobTypeToString(priv-job.asyncJob));
 +while (priv-job.asyncJob) {
 +if (qemuDomainObjEndJob(driver, vm) == 0) {
 +vm = NULL;
 +goto cleanup;
 +}
 +virDomainObjUnlock(vm);
 +
 +nanosleep(ts, NULL);
 +
 +virDomainObjLock(vm);
 +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_ABORT)  0)
 +goto cleanup;
 +
 +if (!virDomainObjIsActive(vm)) {
 +virReportError(VIR_ERR_OPERATION_INVALID,
 +   %s, _(domain is not running));
 +goto endjob;
 +}
 +
 +}
 +
 +ret = 0;
  
  endjob:
  if (qemuDomainObjEndJob(driver, vm) == 0)
 

Re: [libvirt] [PATCH] qemu: Allow the user to specify vendor and product for disk

[Osier Yang]

On 2012年11月08日 16:19, Martin Kletzander wrote:

On 11/08/2012 02:52 AM, Dave Allan wrote:

On Thu, Nov 08, 2012 at 09:28:06AM +0800, Osier Yang wrote:

On 2012年11月08日 05:04, Dave Allan wrote:

On Wed, Nov 07, 2012 at 11:24:43PM +0800, Osier Yang wrote:

On 2012年11月05日 21:34, Martin Kletzander wrote:

On 11/05/2012 08:04 AM, Osier Yang wrote:

QEMU supports to set vendor and product strings for disk since
1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
exposes it with new XML elementsvendorandproductof disk
device.
---
  docs/formatdomain.html.in  |   10 +
  docs/schemas/domaincommon.rng  |   10 +
  src/conf/domain_conf.c |   30 
  src/conf/domain_conf.h |2 +
  src/qemu/qemu_command.c|   29 
  .../qemuxml2argv-disk-scsi-disk-vpd.args   |   13 +++
  .../qemuxml2argv-disk-scsi-disk-vpd.xml|   36 
  tests/qemuxml2argvtest.c   |4 ++
  8 files changed, 134 insertions(+), 0 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c8da33d..cc9e871 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1657,6 +1657,16 @@
  of 16 hexadecimal digits.
  span class='since'Since 0.10.1/span
/dd
+dtcodevendor/code/dt
+ddIf present, this element specifies the vendor of a virtual hard
+disk or CD-ROM device. It's a not more than 8 bytes alphanumeric 
string.


Last sentence doesn't make sense, I suggest changing it to either: It
can't be longer than 8 alphanumeric characters. or simply Maximum 8
alphanumeric characters.


Okay, honestly, I wasn't comfortable with the sentence, but can't
get a better one at that time, :-) I will change your advise a bit:

It must not be longer than 8 alphanumeric characters.


Not to be pedantic, but what happens if you hand it doublebyte
characters?


Ah, good question, though QEMU will truncate the string to 8 bytes
anyway, should we do some translation in libvirt? the problem is
how to map the doublebytes vendors/product to single bytes ones,
is there some public specification available? /me starts to think
if it's snake leg (or breaking fly on the wheel) to do the checking
if it's too heavy.


Mostly I was curious about how the code handled doublebyte characters,
but now that I think about it, the SCSI spec mandates 8 bytes of
ASCII[1], but it sounds like qemu is already enforcing that, so I
think it's fine just to let it be freeform and note the spec's
requirement in the documentation.



I'd say if QEMU starts (and truncates or whatever) with invalid vendor
name, there's no problem for us to leave the responsibility on the user,
but OTOH when QEMU won't like what it's doing and will eventually
fix/change that, our machines may not start.  So if we know the
limitation (and it is very easy one), why don't we just check (and
mention it in the docs) that we accept maximum 8 (16) alphanumeric
characters, checking just [A-Za-z0-9] instead of doing some 'isalnum()'
magic.  Check is easy, QEMU will be always satisfied with the option, I
see it as a win-win.



Not really, QEMU won't be happy anyway if it really changes the limits.
But I agree with adding the checking, as per the SCSI spec mandates
8/16 bytes. We don't need to care about the doublebytes then. And a good
reason for the hecking is error out earlier is always good than QEMU
process fails to start. And it's not likely QEMU will change the limits
to violate the spec.

Regards,
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv3] snapshot: qemu: Add support for external inactive snapshots

[Peter Krempa]

On 11/08/12 01:48, Eric Blake wrote:

On 11/07/2012 04:06 PM, Peter Krempa wrote:

This patch adds support for external disk snapshots of inactive domains.
The snapshot is created by calling using qemu-img by calling:

  qemu-img create -f format_of_snapshot -o
  backing_file=/path/to/src,backing_fmt=format_of_backing_image
  /path/to/snapshot




+qemuDomainSnapshotCreateInactiveExternal(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainSnapshotObjPtr snap,
+ bool reuse)



+/* no-op if reuse is true and file exists and is valid */
+if (reuse) {
+if (stat(snapdisk-file, st)  0) {
+if (errno != ENOENT) {
+virReportSystemError(errno,
+ _(unable to stat snapshot image %s),
+ snapdisk-file);
+goto cleanup;
+}
+} else if (!S_ISBLK(st.st_mode)  st.st_size  0) {
+/* the existing image is reused */
+continue;


Hey, I just realized that the logic I want here (if reuse, then check
that it exists; if !reuse, then check that we aren't nuking unrelated
data) was already run during qemuDomainSnapshotPrepare.  So we can
simplify this - if we got here, then we already know that the file
exists and is ready to reuse, so we can simplify this code.


Yes, looking on the code now again, this was partly done in that way in 
the first version (apart from different reuse flag handling). I 
originally wanted the reuse flag to nuke existing files when re-creating 
the image files when reverting to a snapshot. After seeing your comment 
to that patch, I'll embed the logic to the revert function and will take 
into account snapshots on physical partitions that we can't just unlink.





+
+/* update disk definitions */
+for (i = 0; i  snap-def-ndisks; i++) {
+snapdisk = (snap-def-disks[i]);
+defdisk = vm-def-disks[snapdisk-index];
+
+if (snapdisk-snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+VIR_FREE(defdisk-src);
+if (!(defdisk-src = strdup(snapdisk-file))) {
+/* we cannot rollback here in a sane way */
+virReportOOMError();


True, but OOM is enough of a corner case that I'm not too bothered by this.


+return -1;
+}
+defdisk-format = snapdisk-format;
+}
+}
+
+ret = 0;
+
+cleanup:
+virCommandFree(cmd);
+
+/* unlink images if creation has failed */
+if (ret  0  i  0) {
+for (; i  0; i--) {
+snapdisk = (snap-def-disks[i]);
+if (unlink(snapdisk-file)  0)


Possible NULL deref, if you have a disk with no snapshot earlier in the
array than a disk with an external snapshot.  Also, we don't want to
unlink() pre-existing block devices, so it may make more sense to pay
attention to whether we actually created a file.

ACK with this squashed in:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 17de98e..cea1c2f 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -10677,31 +10677,26 @@
qemuDomainSnapshotCreateInactiveExternal(struct qemud_driver *driver,
  virDomainDiskDefPtr defdisk;
  virCommandPtr cmd = NULL;
  const char *qemuImgPath;
-struct stat st;
+virBitmapPtr created;

  int ret = -1;

  if (!(qemuImgPath = qemuFindQemuImgBinary(driver)))
  return -1;

-for (i = 0; i  snap-def-ndisks; i++) {
+if (!(created = virBitmapNew(snap-def-ndisks))) {
+virReportOOMError();
+return -1;
+}
+
+/* If reuse is true, then qemuDomainSnapshotPrepare already
+ * ensured that the new files exist, and it was up to the user to
+ * create them correctly.  */
+for (i = 0; i  snap-def-ndisks  !reuse; i++) {
  snapdisk = (snap-def-disks[i]);
  defdisk = snap-def-dom-disks[snapdisk-index];
-
-/* no-op if reuse is true and file exists and is valid */
-if (reuse) {
-if (stat(snapdisk-file, st)  0) {
-if (errno != ENOENT) {
-virReportSystemError(errno,
- _(unable to stat snapshot
image %s),
- snapdisk-file);
-goto cleanup;
-}
-} else if (!S_ISBLK(st.st_mode)  st.st_size  0) {
-/* the existing image is reused */
-continue;
-}
-}
+if (snapdisk-snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;

  if (!snapdisk-format)
  snapdisk-format = VIR_STORAGE_FILE_QCOW2;
@@ -10736,6 +10731,9 @@ qemuDomainSnapshotCreateInactiveExternal(struct
qemud_driver *driver,
  /* adds cmd line args: 

Re: [libvirt] [PATCH] qemu: Allow migration to be cancelled at any phase

[Jiri Denemark]

On Thu, Nov 08, 2012 at 11:00:40 +0100, Michal Privoznik wrote:
 On 07.11.2012 23:23, Jiri Denemark wrote:
  
  AbortJob API was never a synchronous one and I don't think we need to change
  it. Not to mention that this code unlocks and locks vm without holding an
  extra reference to it. And even if it did so, it cannot guarantee that the 
  job
  it's checking in the loop is the same one it tried to cancel. It's quite
  unlikely but the original job could have finished and another one could have
  been started while this code was sleeping.
 
 However, AbortJob is documented as synchronous. If current implementation 
 doesn't
 follow docs it is a bug. On the other hand, I don't recall anybody screaming 
 about
 it so far. But that means nothing, right?

IIRC the implementation was never synchronous in which case I think we may
just fix the documentation to match reality :-)

  @@ -1172,6 +1172,12 @@ qemuMigrationUpdateJobStatus(struct qemud_driver 
  *driver,
   }
   priv-job.info.timeElapsed -= priv-job.start;
   
  +if (qemuDomainObjAbortAsyncJobRequested(vm)) {
  +VIR_DEBUG(Migration abort requested. Translating 
  +  status to MIGRATION_STATUS_CANCELLED);
  +status = QEMU_MONITOR_MIGRATION_STATUS_CANCELLED;
  +}
  +
  
  This check seems to be to late and that complicates the code later. If we 
  keep
  qemuDomainAbortJob calling qemuMonitorMigrateCancel in qemuDomainAbortJob, 
  we
  may count on that and check priv.job.asyncAbort before entering the monitor
  to tell QEMU to start migrating in qemuMigrationRun. If the abort is not
  requested by then, it may only happen after we call qemuMonitorMigrateTo* 
  and
  in that case the migration will be properly aborted by
  qemuMonitorMigrateCancel.
  
 
 Not really. The issue I've seen was like this:
 
Thread A   Thread B
 1. start async migration out job
 2.Since job is set, abort it by 
 calling 'migrate_cancel'
 3.return to user
 4. issue 'migrate' on the monitor
 
 And I think your suggestion makes it just harder to hit this race and not 
 really avoid it.
 However with my code, we are guaranteed that 'migrate_cancel' will have an 
 effect.

Oh right, we actually need to check priv.job.asyncAbort after entering the
monitor (since we may be preempted while waiting for entering the monitor) but
still before calling qemuMonitorMigrateTo* that actually starts the monitor.
When we entered the monitor, the AbortJob must have already been there or it
will come after we leave the monitor.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Allow migration to be cancelled at any phase

[Michal Privoznik]

On 08.11.2012 11:49, Jiri Denemark wrote:
 On Thu, Nov 08, 2012 at 11:00:40 +0100, Michal Privoznik wrote:
 On 07.11.2012 23:23, Jiri Denemark wrote:

 AbortJob API was never a synchronous one and I don't think we need to change
 it. Not to mention that this code unlocks and locks vm without holding an
 extra reference to it. And even if it did so, it cannot guarantee that the 
 job
 it's checking in the loop is the same one it tried to cancel. It's quite
 unlikely but the original job could have finished and another one could have
 been started while this code was sleeping.

 However, AbortJob is documented as synchronous. If current implementation 
 doesn't
 follow docs it is a bug. On the other hand, I don't recall anybody screaming 
 about
 it so far. But that means nothing, right?
 
 IIRC the implementation was never synchronous in which case I think we may
 just fix the documentation to match reality :-)
 
 @@ -1172,6 +1172,12 @@ qemuMigrationUpdateJobStatus(struct qemud_driver 
 *driver,
  }
  priv-job.info.timeElapsed -= priv-job.start;
  
 +if (qemuDomainObjAbortAsyncJobRequested(vm)) {
 +VIR_DEBUG(Migration abort requested. Translating 
 +  status to MIGRATION_STATUS_CANCELLED);
 +status = QEMU_MONITOR_MIGRATION_STATUS_CANCELLED;
 +}
 +

 This check seems to be to late and that complicates the code later. If we 
 keep
 qemuDomainAbortJob calling qemuMonitorMigrateCancel in qemuDomainAbortJob, 
 we
 may count on that and check priv.job.asyncAbort before entering the monitor
 to tell QEMU to start migrating in qemuMigrationRun. If the abort is not
 requested by then, it may only happen after we call qemuMonitorMigrateTo* 
 and
 in that case the migration will be properly aborted by
 qemuMonitorMigrateCancel.


 Not really. The issue I've seen was like this:

Thread A  Thread B
 1. start async migration out job
 2.   Since job is set, abort it by 
 calling 'migrate_cancel'
 3.   return to user
 4. issue 'migrate' on the monitor

 And I think your suggestion makes it just harder to hit this race and not 
 really avoid it.
 However with my code, we are guaranteed that 'migrate_cancel' will have an 
 effect.
 
 Oh right, we actually need to check priv.job.asyncAbort after entering the
 monitor (since we may be preempted while waiting for entering the monitor) but
 still before calling qemuMonitorMigrateTo* that actually starts the monitor.
 When we entered the monitor, the AbortJob must have already been there or it
 will come after we leave the monitor.
 
 Jirka
 

Yeah, now that I've traced all possibilities on a paper I see my picture
of situation wasn't quite correct as well. I'll post v2 shortly.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[Jiri Denemark]

On Thu, Nov 08, 2012 at 11:29:39 +0800, li guang wrote:
 Hi, Jiri, Daniel
 Any comment on this patch?

Hi,

Isn't my review I sent less than a day after your v13 enough? See
https://www.redhat.com/archives/libvir-list/2012-November/msg00294.html if you
missed it for whatever reason.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Fix function header formating of 2 functions

[Peter Krempa]

Headers of qemuDomainSnapshotLoad and qemuDomainNetsRestart were
improperly formatted.
---
Pushing under the trivial rule.
---
 src/qemu/qemu_driver.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 79b9607..3309f34 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -445,9 +445,11 @@ err_exit:
 return NULL;
 }

-static void qemuDomainSnapshotLoad(void *payload,
-   const void *name ATTRIBUTE_UNUSED,
-   void *data)
+
+static void
+qemuDomainSnapshotLoad(void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   void *data)
 {
 virDomainObjPtr vm = (virDomainObjPtr)payload;
 char *baseDir = (char *)data;
@@ -559,9 +561,10 @@ cleanup:
 }


-static void qemuDomainNetsRestart(void *payload,
-const void *name ATTRIBUTE_UNUSED,
-void *data ATTRIBUTE_UNUSED)
+static void
+qemuDomainNetsRestart(void *payload,
+  const void *name ATTRIBUTE_UNUSED,
+  void *data ATTRIBUTE_UNUSED)
 {
 int i;
 virDomainObjPtr vm = (virDomainObjPtr)payload;
-- 
1.8.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Gene Czarcinski]

On 11/07/2012 04:23 PM, Doug Goldstein wrote:

On Wed, Nov 7, 2012 at 3:05 PM, Gene Czarcinski g...@czarc.net wrote:

  I have a working patch to have dnsmasq support RA instead of radvd.
However, something has come up and it will be a week to ten days before I
can get it in shape to submit.

The current patch has three variables added to the _virNetworkObj structure:
dnsmasqRA flag and both major and minor values for the dnsmasq's version.

I use dnsmasq --version and then parse out the major/minor version values.
If major2, then dnamsqFA=1.  If major=2 and minor=63, then dnsmasqRA=1.
For all other cases, dnsmasqRA=0.

Code is added to the radvd functions which checks dnsmasqRA and exits if it
is 1.

Code is added to the dnsmasq configuration file if dnsmasqRa=1.  If
dhcp-range or dhcp-hosts is specified for IPv6, then enable-ra is added for
stateful (dhcpv6).  Otherwise, a special
dhcp-range=ipv6-subnet-address,ra-only so that the ManagedFlag will be
off in the RA packets for stateless operation.

OK, how does that sound?  Everyone comfortable with that?

Another thing is that I plan to add a test such that if the radvd executable
is not valid, the dnsmasqRA=1.

As I was doing this, I also looked through the libvirt.spec file. My, what a
wonderful example of wizardly that is.  Anyway, I thought some updates may
be in ortder:

- increase the minimum version for dnsmasq from 2.41 to 2.48.

- why is radvd required for rpmbuild?

- in light of my patch, make radvd an optional runtime requirement. I am not
a spec file expert by any means but there must be a way to not require radvd
if dnsmasq - 2.63.

Comments?

Gene


I'm still not thrilled that you're pushing forward with requiring 2.63
+ a few patches backported from 2.64 into 2.63 and only checking
against 2.63.
I assume you are talking about dhcpv6 and not radvd.  Thus, your 
concerns are related to, if dnsmasq's version = 2.63, then use dnsmasq 
to support both statefull (ManagedFlag on) and stateless (ManagedFlag 
off) IPv6.


DHCPv6 does work in dnsmasq-2.63 but not very well with respect to 
libvirt.  Libvirt plus my patch to add interface= will allow one DHCPv6 
dnsmasq.  With the two patches, everything works as it should.


If you do not specify any dhcp-range= or dhcp-host= parameters, then it 
does not matter and everything works as it should.  If you do specify 
them, then you will (at best) get some indication with a message to 
syslog and, at worst, nothing happens and DHCPv6 requests are ignored.


The good news is that I have gotten the attention of dnsmasq's Fedora 
maintainer and he is looking into it.  I expect an update for Fedora 
real soon now and, since Simon said that dnsmasq-2.64 should be 
available real soon now, that 2.64 be added to F18 updates sooner 
rather than later.


There are currently NO checks as far as the DHCPv6 implementing code.  
If you specify dhcp-range or dhcp-host, the parameters will be passed to 
dnsmasq.  I could add a check, and if the dnsmasq is earlier than 2.63, 
to fail dnsmasq startup.  I could also issue a warning message is 
dnsmasq is 2.63.  I have not done that because, if you do NOT specify 
dhcp-range and/or dhcp-host, nothing will happen and things work as they 
do now.


I noticed that your email has @gentoo,org.  I had a fleeting look at 
gentoo a bunch of years ago.  I do not know what problems result in your 
concern.


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Fix domain ID numbering race condition

[Peter Krempa]

When the libvirt daemon is restarted it tries to reconnect to running
qemu domains. Since commit d38897a5d4b1880e1998394b2a37bba979bbdff1 the
re-connection code runs in separate threads. In the original
implementation the maximum of domain ID's (that is used as an
initializer for numbering guests created next) while libvirt was
reconnecting to the guest.

With the threaded implementation this opens a possibility for race
conditions with the thread that is autostarting guests. When there's a
guest running with id 1 and the daemon is restarted. The autostart code
is reached first and spawns the first guest that should be autostarted
as id 1. This results into the following unwanted situation:

 # virsh list
   IdName   State
  
   1 guest1 running
   1 guest2 running

This patch extracts the detection code before the re-connection threads
are started so that the maximum id of the guests being reconnected to is
known.

The only semantic change created by this is if the guest with greatest ID
quits before we are able to reconnect it's ID is used anyway as the
greatest one as without this patch the greatest ID of a process we could
successfuly reconnect to would be used.
---
 src/qemu/qemu_driver.c  | 21 +
 src/qemu/qemu_process.c |  3 ---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3309f34..8ca8913 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -590,6 +590,20 @@ qemuDomainNetsRestart(void *payload,
 virDomainObjUnlock(vm);
 }

+
+static void
+qemuDomainFindMaxID(void *payload,
+const void *name ATTRIBUTE_UNUSED,
+void *data)
+{
+virDomainObjPtr vm = payload;
+int *driver_maxid = data;
+
+if (vm-def-id = *driver_maxid)
+*driver_maxid = vm-def-id + 1;
+}
+
+
 /**
  * qemudStartup:
  *
@@ -863,6 +877,13 @@ qemudStartup(int privileged) {
 NULL, NULL)  0)
 goto error;

+/* find the maximum ID from active and transient configs to initialize
+ * the driver with. This is to avoid race between autostart and reconnect
+ * threads */
+virHashForEach(qemu_driver-domains.objs,
+   qemuDomainFindMaxID,
+   (qemu_driver-nextvmid));
+
 virHashForEach(qemu_driver-domains.objs, qemuDomainNetsRestart, NULL);

 conn = virConnectOpen(qemu_driver-uri);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d8cf4c3..8bf80e7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3230,9 +3230,6 @@ qemuProcessReconnect(void *opaque)
 goto error;
 }

-if (obj-def-id = driver-nextvmid)
-driver-nextvmid = obj-def-id + 1;
-
 endjob:
 if (!qemuDomainObjEndJob(driver, obj))
 obj = NULL;
-- 
1.8.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2] qemu: Allow migration to be cancelled at prepare phase

[Michal Privoznik]

Currently, if user calls virDomainAbortJob we just issue
'migrate_cancel' and hope for the best. However, if user calls
the API in wrong phase when migration hasn't been started yet
(perform phase) the cancel request is just ignored. With this
patch, the request is remembered and as soon as perform phase
starts, migration is cancelled.
---

diff to v1:
-don't move 'migrate_cancel'
-drop qemuDomainObjAbortAsyncJobRequested()
-detect asyncAbort earlier

 src/qemu/qemu_domain.c|   12 
 src/qemu/qemu_domain.h|2 ++
 src/qemu/qemu_driver.c|1 +
 src/qemu/qemu_migration.c |   11 +++
 4 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a5592b9..e0d6951 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -160,6 +160,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job-mask = DEFAULT_JOB_MASK;
 job-start = 0;
 job-dump_memory_only = false;
+job-asyncAbort = false;
 memset(job-info, 0, sizeof(job-info));
 }
 
@@ -959,6 +960,17 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver, 
virDomainObjPtr obj)
 return virObjectUnref(obj);
 }
 
+void
+qemuDomainObjAbortAsyncJob(virDomainObjPtr obj)
+{
+qemuDomainObjPrivatePtr priv = obj-privateData;
+
+VIR_DEBUG(Requesting abort of async job: %s,
+  qemuDomainAsyncJobTypeToString(priv-job.asyncJob));
+
+priv-job.asyncAbort = true;
+}
+
 static int
 qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
   bool driver_locked,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9c2f67c..a2acc0a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -111,6 +111,7 @@ struct qemuDomainJobObj {
 unsigned long long start;   /* When the async job started */
 bool dump_memory_only;  /* use dump-guest-memory to do dump */
 virDomainJobInfo info;  /* Async job progress data */
+bool asyncAbort;/* abort of async job requested */
 };
 
 typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet;
@@ -204,6 +205,7 @@ bool qemuDomainObjEndJob(struct qemud_driver *driver,
 bool qemuDomainObjEndAsyncJob(struct qemud_driver *driver,
   virDomainObjPtr obj)
 ATTRIBUTE_RETURN_CHECK;
+void qemuDomainObjAbortAsyncJob(virDomainObjPtr obj);
 void qemuDomainObjSetJobPhase(struct qemud_driver *driver,
   virDomainObjPtr obj,
   int phase);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 01ba7eb..9aa8340 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10373,6 +10373,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) {
 }
 
 VIR_DEBUG(Cancelling job at client request);
+qemuDomainObjAbortAsyncJob(vm);
 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorMigrateCancel(priv-mon);
 qemuDomainObjExitMonitor(driver, vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5f8a9c5..0a52486 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2168,6 +2168,17 @@ qemuMigrationRun(struct qemud_driver *driver,
QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;
 
+if (priv-job.asyncAbort) {
+/* explicitly do this *after* we entered the monitor,
+ * as this is a critical section so we are guaranteed
+ * priv-job.asyncAbort will not change */
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+virReportError(VIR_ERR_OPERATION_ABORTED, _(%s: %s),
+   qemuDomainAsyncJobTypeToString(priv-job.asyncJob),
+   _(canceled by client));
+goto cleanup;
+}
+
 if (qemuMonitorSetMigrationSpeed(priv-mon, migrate_speed)  0) {
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 goto cleanup;
-- 
1.7.8.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemu: refactor graphics code to not hardcode a single display

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
 The check for a single display remains so no new functionality is added.
 ---
 
 Concerning both patches, I've tested running with all three
 simultaneously (sdl+spice+vnc) and with spice+vnc, using a
 single qxl device.
 
  src/qemu/qemu_command.c | 2425 
 ---
  src/qemu/qemu_process.c |   70 +-
  2 files changed, 1259 insertions(+), 1236 deletions(-)

I'm not really sure what has happened here, but you have got
an absolutely enourmous diff here. I would expect this patch
to only affect the small part of qemuBuildCommandLine that
actually deals with graphics devices. As it is now, it is
impossible to review this patch I'm afraid :-(

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemu: refactor graphics code to not hardcode a single display

[Peter Krempa]

On 11/08/12 15:04, Daniel P. Berrange wrote:

On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:

The check for a single display remains so no new functionality is added.
---

Concerning both patches, I've tested running with all three
simultaneously (sdl+spice+vnc) and with spice+vnc, using a
single qxl device.

  src/qemu/qemu_command.c | 2425 ---
  src/qemu/qemu_process.c |   70 +-
  2 files changed, 1259 insertions(+), 1236 deletions(-)


I'm not really sure what has happened here, but you have got
an absolutely enourmous diff here. I would expect this patch
to only affect the small part of qemuBuildCommandLine that
actually deals with graphics devices. As it is now, it is
impossible to review this patch I'm afraid :-(

Daniel



It was a code split, where git diff got mad. With the patience algorithm 
the patch looks much better.


Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] AbortJob: Fix documentation

[Michal Privoznik]

This API was never synchronous and probably doesn't even need to be.
---

I am sending this as a separate patch as regardless if my
previous patch got accepted or not this one is still true.

 src/libvirt.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index bcb8233..bdb1dc6 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17182,8 +17182,7 @@ error:
  * @domain: a domain object
  *
  * Requests that the current background job be aborted at the
- * soonest opportunity. This will block until the job has
- * either completed, or aborted.
+ * soonest opportunity.
  *
  * Returns 0 in case of success and -1 in case of failure.
  */
-- 
1.7.8.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Expose virDomainGetAutostart through virsh?

[Daniel P. Berrange]

On Wed, Nov 07, 2012 at 11:32:22AM +0100, Peter Krempa wrote:
 On 11/07/12 11:04, Ruben Kerkhof wrote:
 Hi list,
 
 I'd like to check if a domain has autostart enabled. I do this now by
 looking if there's a symlink in /etc/libvirt/qemu/autostart, but it
 feels a bit hackish.
 
 Is this something that could be added to virsh?
 Something like virsh get-autostart domain would be great.
 
 For now there are two options how to check the autostart flag:
 
 1) virsh dominfo - This is suitable to check for the state of a
 single guest. Unfortunately we have just this one output option
 where it is embedded with other information about the guest:
 
 $ virsh dominfo tr
 Id: 1
 Name:   tr
 UUID:   17f42b42-9fdd-81e3-4a93-a75021a707d3
 OS Type:hvm
 State:  running
 CPU(s): 1
 CPU time:   200.5s
 Max memory: 53248 KiB
 Used memory:53248 KiB
 Persistent: yes
 Autostart:  enable- here!
 Managed save:   no
 Security model: none
 Security DOI:   0
 
 
 2) use virsh list --all --autostart to list guests that have the
 autostart flag enabled (also note that there are script-friendly
 outputs for virsh list --name and --uuid):
 
 $ virsh list --all --autostart
  IdName   State
 
  1 tr running
  - Bare   shut off
 
 
 We're planing on adding the autostart flag as an XML element and a
 few other improvements of autostaring guests.

I don't think the XML should be in the business of having the
autostart flag, as this is not guest configuration information.
Having it in the XML introduces the problem of maintaining the
correct synchronization between the autostart symlink and the
XML description.

Further, in the future I'd like to actually replace our current
autostart code, with code that just sets up systemd units to
deal with autostart.

In such a scenario, the user will be able to toggle autostart
directly using systemd, so we don't want libvirt duplicating
that info in the XML since it won't be aware of when systemd
changes the autostart flag.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Daniel P. Berrange]

On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
 the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
 'start', 'stop', 'restart'; might be easier to remember than the
 current mix of 'start', 'shutdown', 'reboot'.
 
 * tools/virsh-domain.c (domManagementCmds): Add other command names.
 * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
 ---
 
 This patch documents both spellings.  An alternative would be to
 leave the alternate spellings as hidden aliases (virsh has support
 for that), but still mention them in virsh.pod (see how we did an
 alias for nodedev-dettach, for reference).

NACK to this patch. I think the current command names are good.
Creating duplicates will make life worse. First, it creates
divergance from the similarly named commands for networks,
storage and other objects. It also means scripts written again
the new commands will not work with existing libvirt.

I actually think that shutdown  reboot are *better* names
than restart and stop.

If we wanted to replace any existing names, then the 'create'
and 'destroy' names are the ones to replace, and for those I
would expect to use 'boot' and 'stop'. I still don't thin
we should do that either, due to creating inconsistency with
other commands.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] qemu: Build command line for ivshmem device

[Daniel P. Berrange]

On Wed, Nov 07, 2012 at 05:40:44PM +0800, Osier Yang wrote:
 Depends on whether source path is specified, the command line
 is built like:
 
   * 'path' is specified, with interrupts (/tmp/nahanni is the
  ivshmem server socket path)
 -chardev socket,path=/tmp/nahanni,id=nahanni
 -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on
 
   * 'path' is not specified, without interrupts
 -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on
 
 * src/qemu/qemu_command.c (New helper qemuBuildMemoryDevStr to
to build args of '-device'; Assign
PCI address for the device; Build
args of '-chardev')
 * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml: (See below)
 * tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args: (See below)
 * tests/qemuxml2argvtest.c: (Add tests)

 ---
  src/qemu/qemu_command.c  |   85 
 ++
  tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args |7 ++
  tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml  |   33 +
  tests/qemuxml2argvtest.c |2 +
  4 files changed, 127 insertions(+), 0 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml

No changes to SELinux ?  I'm reasonably sure the policy should
forbid QEMU from creating the shared memory backing file. For
hugepages, we had to create a per-QEMU directory, label that and
then pass it to QEMU.

This perhaps implies that our XML should not contain the actual
path. Libvirt can just create a path based on the name of the
device and set a label of the dir as for hugepages. 


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] docs: Add documents for memory device

[Daniel P. Berrange]

On Wed, Nov 07, 2012 at 05:40:41PM +0800, Osier Yang wrote:
 Considering there could be other memory related devices in future,
 this introduces a general device model, called memory device,
 instead of a specific device like ivshmem, currently only
 ivshmem is supported though.
 
 An example of the XML:
 
   memory model='ivshmem'

Since we have many sub-elements, I think it is preferrable to
also have the model in a subelement as we do with things like
disk/network/etc

 source id='nahanni' path='/tmp/nahanni'/

Use '/dev/shm' as the example path since that's more usual

 size unit='M'512/size
 vectors8/vectors
 /ioeventfd

Typo with the '/' location

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] v6-7: Add support for DHCPv6

[Gene Czarcinski]

This support includes IPv6 dhcp-range= and dhcp-host=
for one IPv6 subnetwork on one interface.

The parameter tests have been updated to add some new
DHCPv6 tests.  Three new tests (6 files) were added.

The xml format html documentation has been updated to
reflect support for IPv6 DHCP.

v6-7: This patch include a bugfix for ipv4flag and ipv6flag
initialization.
---
 docs/formatnetwork.html.in | 108 ++-
 src/conf/network_conf.c| 100 ---
 src/network/bridge_driver.c| 326 +
 src/util/dnsmasq.c |   9 +-
 tests/networkxml2argvdata/dhcp6-network.argv   |  16 +
 tests/networkxml2argvdata/dhcp6-network.xml|  16 +
 tests/networkxml2argvdata/isolated-network.argv|   2 +-
 tests/networkxml2argvdata/nat-network-dhcp6.argv   |  19 ++
 tests/networkxml2argvdata/nat-network-dhcp6.xml|  26 ++
 .../nat-network-dns-srv-record-minimal.argv|   2 +-
 .../nat-network-dns-srv-record.argv|   2 +-
 .../nat-network-dns-txt-record.argv|   2 +-
 tests/networkxml2argvdata/nat-network.argv |   2 +-
 tests/networkxml2argvdata/netboot-network.argv |   8 +-
 .../networkxml2argvdata/netboot-proxy-network.argv |   4 +-
 .../routed-network-dhcphost.argv   |  14 +
 .../routed-network-dhcphost.xml|  19 ++
 tests/networkxml2argvtest.c|   3 +
 18 files changed, 498 insertions(+), 180 deletions(-)
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.argv
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.xml
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.argv
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.xml
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.argv
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.xml

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..b91672a 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -577,8 +577,10 @@
 dotted-decimal format, or an IPv6 address in standard
 colon-separated hexadecimal format, that will be configured on
 the bridge
-device associated with the virtual network. To the guests this
-address will be their default route. For IPv4 addresses, the 
codenetmask/code
+device associated with the virtual network. To the guests this IPv4
+address will be their IPv4 default route.  For IPv6, the default route 
is
+established via Router Advertisement.
+For IPv4 addresses, the codenetmask/code
 attribute defines the significant bits of the network address,
 again specified in dotted-decimal format.  For IPv6 addresses,
 and as an alternate method for IPv4 addresses, you can specify
@@ -587,10 +589,13 @@
 could also be given as codeprefix='24'/code. The 
codefamily/code
 attribute is used to specify the type of address - 'ipv4' or 'ipv6'; 
if no
 codefamily/code is given, 'ipv4' is assumed. A network can have 
more than
-one of each family of address defined, but only a single address can 
have a
-codedhcp/code or codetftp/code element. span 
class=sinceSince 0.3.0;
+one of each family of address defined, but only a single IPv4 address 
can have a
+codedhcp/code or codetftp/code element. span 
class=sinceSince 0.3.0 /span
 IPv6, multiple addresses on a single network, codefamily/code, and
-codeprefix/code since 0.8.7/span
+codeprefix/code. span class=sinceSince 0.8.7/span  In 
addition
+to the one IPv4 address which has a codedhcp/code definition, one 
IPv6
+address can have a codedhcp/code definition.
+span class=since Since 1.0.1/span
 dl
   dtcodetftp/code/dt
   ddImmediately within
@@ -611,27 +616,46 @@
 codedhcp/code element is not supported for IPv6, and
 is only supported on a single IP address per network for IPv4.
 span class=sinceSince 0.3.0/span
+The codedhcp/code element is now supported for IPv6.
+Again, there is a restriction that only one IPv6 address definition
+is able to have a codedhcp/code element.
+span class=sinceSince 1.0.1/span
 dl
   dtcoderange/code/dt
   ddThe codestart/code and codeend/code attributes on the
 coderange/code element specify the boundaries of a pool of
-IPv4 addresses to be provided to DHCP clients. These two 
addresses
+addresses to be provided to DHCP clients. These two addresses
 must lie within the scope of the network defined on the parent
-codeip/code element.  span class=sinceSince 
0.3.0/span
+codeip/code element. 

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Daniel P. Berrange]

On Tue, Nov 06, 2012 at 09:30:15AM +0100, Viktor Mihajlovski wrote:
 On 11/05/2012 08:59 PM, Eric Blake wrote:
 
 This patch documents both spellings.  An alternative would be to
 leave the alternate spellings as hidden aliases (virsh has support
 for that), but still mention them in virsh.pod (see how we did an
 alias for nodedev-dettach, for reference).
 In my opinion start, stop, restart is better than the mix before and
 semantically more accurate than boot, shutdown, reboot. The latter
 uses terms that apply to the operating system inside the domain
 rather than the domain (virtual machine) itself.

The 'shutdown' and 'reboot' commands use terms that apply to the
operating system, because they *are* controlling the operating
system, not the virtual machine. They directly result in the
guest OS commands 'shutdown' and 'reboot' being invoked.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemu: refactor graphics code to not hardcode a single display

[Alon Levy]

 On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
  The check for a single display remains so no new functionality is
  added.
  ---
  
  Concerning both patches, I've tested running with all three
  simultaneously (sdl+spice+vnc) and with spice+vnc, using a
  single qxl device.
  
   src/qemu/qemu_command.c | 2425
   ---
   src/qemu/qemu_process.c |   70 +-
   2 files changed, 1259 insertions(+), 1236 deletions(-)
 
 I'm not really sure what has happened here, but you have got
 an absolutely enourmous diff here. I would expect this patch
 to only affect the small part of qemuBuildCommandLine that
 actually deals with graphics devices. As it is now, it is
 impossible to review this patch I'm afraid :-(

This is simply a refactoring, I've extracted the whole block inside 
qemuBuildCommandLine dealing with the graphics elements that qemu supports 
(vnc, sdl, spice) to qemuBuildGraphicsCommandLine. git diff is not very 
viewable since it looks at too few context lines, maybe it can be tweaked to 
give an easier view.

 
 Daniel
 --
 |: http://berrange.com  -o-
 |   http://www.flickr.com/photos/dberrange/ :|
 |: http://libvirt.org  -o-
 |http://virt-manager.org :|
 |: http://autobuild.org   -o-
 |http://search.cpan.org/~danberr/ :|
 |: http://entangle-photo.org   -o-
 |  http://live.gnome.org/gtk-vnc :|
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Expose virDomainGetAutostart through virsh?

[Peter Krempa]

On 11/08/12 15:13, Daniel P. Berrange wrote:

On Wed, Nov 07, 2012 at 11:32:22AM +0100, Peter Krempa wrote:

On 11/07/12 11:04, Ruben Kerkhof wrote:

Hi list,

I'd like to check if a domain has autostart enabled. I do this now by
looking if there's a symlink in /etc/libvirt/qemu/autostart, but it
feels a bit hackish.

Is this something that could be added to virsh?
Something like virsh get-autostart domain would be great.


For now there are two options how to check the autostart flag:

1) virsh dominfo - This is suitable to check for the state of a
single guest. Unfortunately we have just this one output option
where it is embedded with other information about the guest:

$ virsh dominfo tr
Id: 1
Name:   tr
UUID:   17f42b42-9fdd-81e3-4a93-a75021a707d3
OS Type:hvm
State:  running
CPU(s): 1
CPU time:   200.5s
Max memory: 53248 KiB
Used memory:53248 KiB
Persistent: yes
Autostart:  enable  - here!
Managed save:   no
Security model: none
Security DOI:   0


2) use virsh list --all --autostart to list guests that have the
autostart flag enabled (also note that there are script-friendly
outputs for virsh list --name and --uuid):

$ virsh list --all --autostart
  IdName   State

  1 tr running
  - Bare   shut off


We're planing on adding the autostart flag as an XML element and a
few other improvements of autostaring guests.


I don't think the XML should be in the business of having the
autostart flag, as this is not guest configuration information.
Having it in the XML introduces the problem of maintaining the
correct synchronization between the autostart symlink and the
XML description.


Yes, that will be troublesome. It's unfortunate we still have to use the 
symlink approach.




Further, in the future I'd like to actually replace our current
autostart code, with code that just sets up systemd units to
deal with autostart.

In such a scenario, the user will be able to toggle autostart
directly using systemd, so we don't want libvirt duplicating
that info in the XML since it won't be aware of when systemd
changes the autostart flag.


Note that there are still distros that are doing everything possible to 
avoid having systemd. It wouldn't be nice if we broke autostarting there.




Daniel



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v7 5/6] make /proc/meminfo isolate with host through fuse

[Daniel P. Berrange]

On Tue, Nov 06, 2012 at 09:00:36AM +, Richard W.M. Jones wrote:
 On Tue, Nov 06, 2012 at 02:07:21PM +0800, Gao feng wrote:
  +while (fgets(line, sizeof(line), statfd) != NULL) {
 
 I wonder if libvirt prefers the getline API instead of fgets?

I don't think we have a documented policy for this, but getline
is a nicer API IMHO

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Expose virDomainGetAutostart through virsh?

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 03:41:12PM +0100, Peter Krempa wrote:
 On 11/08/12 15:13, Daniel P. Berrange wrote:
 On Wed, Nov 07, 2012 at 11:32:22AM +0100, Peter Krempa wrote:
 On 11/07/12 11:04, Ruben Kerkhof wrote:
 Hi list,
 
 I'd like to check if a domain has autostart enabled. I do this now by
 looking if there's a symlink in /etc/libvirt/qemu/autostart, but it
 feels a bit hackish.
 
 Is this something that could be added to virsh?
 Something like virsh get-autostart domain would be great.
 
 For now there are two options how to check the autostart flag:
 
 1) virsh dominfo - This is suitable to check for the state of a
 single guest. Unfortunately we have just this one output option
 where it is embedded with other information about the guest:
 
 $ virsh dominfo tr
 Id: 1
 Name:   tr
 UUID:   17f42b42-9fdd-81e3-4a93-a75021a707d3
 OS Type:hvm
 State:  running
 CPU(s): 1
 CPU time:   200.5s
 Max memory: 53248 KiB
 Used memory:53248 KiB
 Persistent: yes
 Autostart:  enable  - here!
 Managed save:   no
 Security model: none
 Security DOI:   0
 
 
 2) use virsh list --all --autostart to list guests that have the
 autostart flag enabled (also note that there are script-friendly
 outputs for virsh list --name and --uuid):
 
 $ virsh list --all --autostart
   IdName   State
 
   1 tr running
   - Bare   shut off
 
 
 We're planing on adding the autostart flag as an XML element and a
 few other improvements of autostaring guests.
 
 I don't think the XML should be in the business of having the
 autostart flag, as this is not guest configuration information.
 Having it in the XML introduces the problem of maintaining the
 correct synchronization between the autostart symlink and the
 XML description.
 
 Yes, that will be troublesome. It's unfortunate we still have to use
 the symlink approach.
 
 
 Further, in the future I'd like to actually replace our current
 autostart code, with code that just sets up systemd units to
 deal with autostart.
 
 In such a scenario, the user will be able to toggle autostart
 directly using systemd, so we don't want libvirt duplicating
 that info in the XML since it won't be aware of when systemd
 changes the autostart flag.
 
 Note that there are still distros that are doing everything possible
 to avoid having systemd. It wouldn't be nice if we broke
 autostarting there.

Of course, not least RHEL-5 and RHEL-6. If we did support systemd,
we would have a compile time option to use systemd vs our current
autostart code.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 11:29:39AM +0800, li guang wrote:
 Hi, Jiri, Daniel
 Any comment on this patch?

Looks like you fixed all the issues I've brought up now. I'll
defer to Jiri for review of all the other issues he identified.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] dnsmasq conf-file, interface=, and DHCPv6

[Gene Czarcinski]

On 11/06/2012 11:11 AM, Gene Czarcinski wrote:

   v6-6: put dnsmasq parameters into a file
   v6-6: add dnsmasq interface= parameter
   v6-6: Add support for DHCPv6
I will be adding some additional patches which will in addition to the 
patches already submitted.


I am attempting to keep them all on the same thread since they are all 
somewhat related and there is a definite order and dependence in 
application.


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] S390: Add SCLP console front end support

[Viktor Mihajlovski]

From: J.B. Joret j...@linux.vnet.ibm.com

The SCLP console is the native console type for s390 and is preferred
over the virtio console as it doesn't require special drivers and
is more efficient. Recent versions of QEMU come with SCLP support
which is hereby enabled.

The new target types 'sclp' and 'sclplm' can be used to specify a
SCLP console. Adding documentation, domain schema and XML processing
support.

Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 docs/formatdomain.html.in | 19 ++-
 docs/schemas/domaincommon.rng |  2 ++
 src/conf/domain_conf.c|  4 +++-
 src/conf/domain_conf.h|  2 ++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c8da33d..e68ab81 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3591,7 +3591,7 @@ qemu-kvm -net nic,model=? /dev/null
 /p
 
 ul
-  liIf no codetargetType/code attribue is set, then the default
+  liIf no codetargetType/code attribute is set, then the default
 device type is according to the hypervisor's rules. The default
 type will be added when re-querying the XML fed into libvirt.
 For fully virtualized guests, the default device type will usually
@@ -3605,6 +3605,12 @@ qemu-kvm -net nic,model=? /dev/null
   liOnly the first codeconsole/code element may use a 
codetargetType/code
 of codeserial/code. Secondary consoles must all be paravirtualized.
   /li
+  liOn s390, the codeconsole/code element may use a
+codetargetType/code of codesclp/code or codesclplm/code
+(line mode). SCLP is the native console type for s390. There's no
+controller associated to SCLP consoles.
+span class=sinceSince 1.0.1/span
+  /li
 /ul
 
 p
@@ -3630,6 +3636,17 @@ qemu-kvm -net nic,model=? /dev/null
   lt;/devicesgt;
   .../pre
 
+pre
+  ...
+  lt;devicesgt;
+lt;!-- KVM s390 sclp console --gt;
+lt;console type='pty'gt;
+  lt;source path='/dev/pts/1'/gt;
+  lt;target type='sclp' port='0'/gt;
+lt;/consolegt;
+  lt;/devicesgt;
+  .../pre
+
 p
   If the console is presented as a serial port, the codetarget/code
   element has the same attributes as for a serial port. There is usually
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2beb035..7a9c93d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2406,6 +2406,8 @@
 valuevirtio/value
 valuelxc/value
 valueopenvz/value
+valuesclp/value
+valuesclplm/value
   /choice
 /attribute
   /define
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0575fcd..e105a02 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -344,7 +344,9 @@ VIR_ENUM_IMPL(virDomainChrConsoleTarget,
   uml,
   virtio,
   lxc,
-  openvz)
+  openvz,
+  sclp,
+  sclplm)
 
 VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
   parallel,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6539281..73b4313 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -892,6 +892,8 @@ enum virDomainChrConsoleTargetType {
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO,
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC,
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM,
 
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
 };
-- 
1.7.12.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] S390: Enable SCLP Console in QEMU driver

[Viktor Mihajlovski]

From: J.B. Joret j...@linux.vnet.ibm.com

This is the QEMU backend code for the SCLP console support.
It includes SCLP capability detection, QEMU command line generation
and a test case.

Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 src/qemu/qemu_capabilities.c   |  3 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 59 ++
 .../qemuxml2argv-console-sclp.args |  8 +++
 .../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24 +
 tests/qemuxml2argvtest.c   |  3 ++
 6 files changed, 98 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 97b0b24..1859f53 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -191,6 +191,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   vnc,
 
   drive-mirror, /* 115 */
+  s390-sclp,
+
 );
 
 struct _qemuCaps {
@@ -1273,6 +1275,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = {
 { usb-hub, QEMU_CAPS_USB_HUB },
 { ich9-ahci, QEMU_CAPS_ICH9_AHCI },
 { virtio-blk-s390, QEMU_CAPS_VIRTIO_S390 },
+{ sclpconsole, QEMU_CAPS_SCLP_S390 },
 { lsi53c895a, QEMU_CAPS_SCSI_LSI },
 { virtio-scsi-pci, QEMU_CAPS_VIRTIO_SCSI_PCI },
 { spicevmc, QEMU_CAPS_DEVICE_SPICEVMC },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index fb88aa1..025fc41 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -153,6 +153,7 @@ enum qemuCapsFlags {
 QEMU_CAPS_BLOCK_COMMIT   = 113, /* block-commit */
 QEMU_CAPS_VNC= 114, /* Is -vnc available? */
 QEMU_CAPS_DRIVE_MIRROR   = 115, /* drive-mirror monitor command */
+QEMU_CAPS_SCLP_S390  = 116, /* -device sclp* */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1e96982..0fd7b16 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3901,6 +3901,37 @@ error:
 return NULL;
 }
 
+static char *qemuBuildSclpDevStr(virDomainChrDefPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE) {
+switch (dev-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+virBufferAddLit(buf, sclpconsole);
+break;
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+virBufferAddLit(buf, sclplmconsole);
+break;
+}
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Cannot use slcp with devices other than console));
+goto error;
+}
+virBufferAsprintf(buf, ,chardev=char%s,id=%s,
+  dev-info.alias, dev-info.alias);
+if (virBufferError(buf)) {
+virReportOOMError();
+goto error;
+}
+
+return virBufferContentAndReset(buf);
+
+error:
+virBufferFreeAndReset(buf);
+return NULL;
+}
+
 static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5804,6 +5835,34 @@ qemuBuildCommandLine(virConnectPtr conn,
 char *devstr;
 
 switch (console-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(sclp console requires QEMU to support 
-device));
+goto error;
+}
+if (!qemuCapsGet(caps, QEMU_CAPS_SCLP_S390)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(sclp console requires QEMU to support 
s390-sclp));
+goto error;
+}
+
+virCommandAddArg(cmd, -chardev);
+if (!(devstr = qemuBuildChrChardevStr(console-source,
+  console-info.alias,
+  caps)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildSclpDevStr(console)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+break;
+
 case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
 if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
diff --git 

[libvirt] [PATCH 0/2] S390: Adding support for SCLP Console

[Viktor Mihajlovski]

The S390 architecture comes with a native console type (SCLP
console) which is now also supported by current QEMU.

This series is enabling libvirt to configure S390 domains with SCLP
consoles.

The domain XML has to be extended for the new console target types
'sclp' and 'sclplm' (line mode = dumb).

As usual the QEMU driver must do capability probing in order to find
out whether SCLP is supported and format the QEMU command line
for the new console type.

J.B. Joret (2):
  S390: Add SCLP console front end support
  S390: Enable SCLP Console in QEMU driver

 docs/formatdomain.html.in  | 19 ++-
 docs/schemas/domaincommon.rng  |  2 +
 src/conf/domain_conf.c |  4 +-
 src/conf/domain_conf.h |  2 +
 src/qemu/qemu_capabilities.c   |  3 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 59 ++
 .../qemuxml2argv-console-sclp.args |  8 +++
 .../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24 +
 tests/qemuxml2argvtest.c   |  3 ++
 10 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml

-- 
1.7.12.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Doug Goldstein]

On Thu, Nov 8, 2012 at 7:08 AM, Gene Czarcinski g...@czarc.net wrote:
 On 11/07/2012 04:23 PM, Doug Goldstein wrote:

 On Wed, Nov 7, 2012 at 3:05 PM, Gene Czarcinski g...@czarc.net wrote:

   I have a working patch to have dnsmasq support RA instead of radvd.
 However, something has come up and it will be a week to ten days before I
 can get it in shape to submit.

 The current patch has three variables added to the _virNetworkObj
 structure:
 dnsmasqRA flag and both major and minor values for the dnsmasq's version.

 I use dnsmasq --version and then parse out the major/minor version
 values.
 If major2, then dnamsqFA=1.  If major=2 and minor=63, then dnsmasqRA=1.
 For all other cases, dnsmasqRA=0.

 Code is added to the radvd functions which checks dnsmasqRA and exits if
 it
 is 1.

 Code is added to the dnsmasq configuration file if dnsmasqRa=1.  If
 dhcp-range or dhcp-hosts is specified for IPv6, then enable-ra is added
 for
 stateful (dhcpv6).  Otherwise, a special
 dhcp-range=ipv6-subnet-address,ra-only so that the ManagedFlag will
 be
 off in the RA packets for stateless operation.

 OK, how does that sound?  Everyone comfortable with that?

 Another thing is that I plan to add a test such that if the radvd
 executable
 is not valid, the dnsmasqRA=1.

 As I was doing this, I also looked through the libvirt.spec file. My,
 what a
 wonderful example of wizardly that is.  Anyway, I thought some updates
 may
 be in ortder:

 - increase the minimum version for dnsmasq from 2.41 to 2.48.

 - why is radvd required for rpmbuild?

 - in light of my patch, make radvd an optional runtime requirement. I am
 not
 a spec file expert by any means but there must be a way to not require
 radvd
 if dnsmasq - 2.63.

 Comments?

 Gene


 I'm still not thrilled that you're pushing forward with requiring 2.63
 + a few patches backported from 2.64 into 2.63 and only checking
 against 2.63.

 I assume you are talking about dhcpv6 and not radvd.  Thus, your concerns
 are related to, if dnsmasq's version = 2.63, then use dnsmasq to support
 both statefull (ManagedFlag on) and stateless (ManagedFlag off) IPv6.

 DHCPv6 does work in dnsmasq-2.63 but not very well with respect to libvirt.
 Libvirt plus my patch to add interface= will allow one DHCPv6 dnsmasq.  With
 the two patches, everything works as it should.

 If you do not specify any dhcp-range= or dhcp-host= parameters, then it does
 not matter and everything works as it should.  If you do specify them, then
 you will (at best) get some indication with a message to syslog and, at
 worst, nothing happens and DHCPv6 requests are ignored.

 The good news is that I have gotten the attention of dnsmasq's Fedora
 maintainer and he is looking into it.  I expect an update for Fedora real
 soon now and, since Simon said that dnsmasq-2.64 should be available real
 soon now, that 2.64 be added to F18 updates sooner rather than later.

 There are currently NO checks as far as the DHCPv6 implementing code.  If
 you specify dhcp-range or dhcp-host, the parameters will be passed to
 dnsmasq.  I could add a check, and if the dnsmasq is earlier than 2.63, to
 fail dnsmasq startup.  I could also issue a warning message is dnsmasq is
 2.63.  I have not done that because, if you do NOT specify dhcp-range and/or
 dhcp-host, nothing will happen and things work as they do now.

 I noticed that your email has @gentoo,org.  I had a fleeting look at gentoo
 a bunch of years ago.  I do not know what problems result in your concern.

 Gene


I hardly see what me having any involvement in Gentoo has anything to
do with this short of saying Gentoo isn't for everyone, just as
Debian, Ubuntu, Fedora, or SuSE aren't for everyone and Gentoo wasn't
your cup of tea. My point is if you're going to add a check for 2.63
but really require 2.63 + 3 patches that Fedora has backported into
their 2.63 version which was your original proposal, this would cause
lots of headaches for every other distro out there unless they
backported those very same patches into 2.63. So better to wait for
2.64 and go forward from there. libvirt works on and targets many more
systems than Fedora.

-- 
Doug Goldstein

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] storage: allow metadata preallocation when creating qcow2 images

[Ján Tomko]

Currently the 'allocation' element is not used when creating new images
with qemu-img. This patch interprets a non-zero value as a request to
preallocate metadata when a qcow2 image is created.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=684793
---
 src/storage/storage_backend.c |   19 +--
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 41a19a1..bf6f7cc 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -669,10 +669,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
 int imgformat = -1;
 virCommandPtr cmd = NULL;
 bool do_encryption = (vol-target.encryption != NULL);
+bool preallocate = false;
 unsigned long long int size_arg;
 
 virCheckFlags(0, -1);
 
+/* qcow2: preallocate metadata if requested allocation is non-zero */
+if (vol-allocation  0  vol-target.format == VIR_STORAGE_FILE_QCOW2)
+preallocate = true;
+
 const char *type = virStorageFileFormatTypeToString(vol-target.format);
 const char *backingType = vol-backingStore.path ?
 virStorageFileFormatTypeToString(vol-backingStore.format) : NULL;
@@ -842,12 +847,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
  vol-target.path, NULL);
 virCommandAddArgFormat(cmd, %lluK, size_arg);
 
-if (do_encryption) {
-if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
-virCommandAddArgList(cmd, -o, encryption=on, NULL);
-} else {
-virCommandAddArg(cmd, -e);
-}
+if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS 
+(do_encryption || preallocate)) {
+virCommandAddArg(cmd, -o);
+virCommandAddArgFormat(cmd, %s%s%s, do_encryption ? 
encryption=on : ,
+   (do_encryption  preallocate) ? , : ,
+   preallocate ? preallocation=metadata : 
);
+} else if (do_encryption){
+virCommandAddArg(cmd, -e);
 }
 }
 
-- 
1.7.8.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Viktor Mihajlovski]

On 11/08/2012 03:25 PM, Daniel P. Berrange wrote:



The 'shutdown' and 'reboot' commands use terms that apply to the
operating system, because they *are* controlling the operating
system, not the virtual machine. They directly result in the
guest OS commands 'shutdown' and 'reboot' being invoked.


At least for QEMU this is only true for agent mode. Otherwise (default)
both virsh shutdown and virsh reboot are the same from the OS
perspective, namely a system powerdown request and will typically
result in the shutdown script to be run.
That's what can confuse users without insight to libvirt code.

--

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research  Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[James B. Byrne]

On Thu, November 8, 2012 09:24, Daniel P. Berrange wrote:


 NACK to this patch. I think the current command names are good.
 Creating duplicates will make life worse. First, it creates
 divergance from the similarly named commands for networks,
 storage and other objects. It also means scripts written again
 the new commands will not work with existing libvirt.

Duplicates (aliases) will make life worse for whom?  In what way?  On
what evidence?  The present nomenclature is idiomatic to virsh and is
a variance with how many people think of managing a host whether
virtual or not.  On the other hand, one does not customarily speak of
rebooting a network or a storage array, at least not in my experience.


 I actually think that shutdown  reboot are *better* names
 than restart and stop.

Then change start to boot and be done with it.  But, the issue really
is what English words are commonly associated with each other in the
context we are dealing with.  I submit that 'start' is not intuitively
associated with 'shutdown' by the vast majority of English speakers
and hardly associated with 'reboot' by any.

Consider the syntax of 'initctl' and 'service'.  Initctl uses start,
stop and restart.  Service scripts virtually without exception use
start, stop and restart, including that for libvirtd.  Operators are
far more likely to be familiar with this combination of terms than any
other.  Why force them to learn yet one more variant?  What is the
advantage for the users?
.

-- 
***  E-Mail is NOT a SECURE channel  ***
James B. Byrnemailto:byrn...@harte-lyne.ca
Harte  Lyne Limited  http://www.harte-lyne.ca
9 Brockley Drive  vox: +1 905 561 1241
Hamilton, Ontario fax: +1 905 561 0757
Canada  L8E 3C3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Dave Allan]

On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
 On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
  the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
  'start', 'stop', 'restart'; might be easier to remember than the
  current mix of 'start', 'shutdown', 'reboot'.
  
  * tools/virsh-domain.c (domManagementCmds): Add other command names.
  * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
  ---
  
  This patch documents both spellings.  An alternative would be to
  leave the alternate spellings as hidden aliases (virsh has support
  for that), but still mention them in virsh.pod (see how we did an
  alias for nodedev-dettach, for reference).
 
 NACK to this patch. I think the current command names are good.
 Creating duplicates will make life worse. First, it creates
 divergance from the similarly named commands for networks,
 storage and other objects. It also means scripts written again
 the new commands will not work with existing libvirt.
 
 I actually think that shutdown  reboot are *better* names
 than restart and stop.
 
 If we wanted to replace any existing names, then the 'create'
 and 'destroy' names are the ones to replace, and for those I
 would expect to use 'boot' and 'stop'. I still don't thin
 we should do that either, due to creating inconsistency with
 other commands.

IMO this is fundamentally an aesthetic question here that's not going
to be solved by debate, however, from the comments so far, the patch
seems to have a fair amount of support.  DV, perhaps you could weigh
in with your opinion?

Dave

 Daniel
 -- 
 |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
 |: http://libvirt.org  -o- http://virt-manager.org :|
 |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
 |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/4] qemu: Build command line for ivshmem device

[Osier Yang]

On 2012年11月08日 22:18, Daniel P. Berrange wrote:

On Wed, Nov 07, 2012 at 05:40:44PM +0800, Osier Yang wrote:

Depends on whether source path is specified, the command line
is built like:

   * 'path' is specified, with interrupts (/tmp/nahanni is the
  ivshmem server socket path)
 -chardev socket,path=/tmp/nahanni,id=nahanni
 -device ivshmem,chardev=nahanni,size=512m,vectors=8,ioeventfd=on

   * 'path' is not specified, without interrupts
 -device ivshmem,shm=nahanni,size=512m,vectors=8,ioeventfd=on

* src/qemu/qemu_command.c (New helper qemuBuildMemoryDevStr to
to build args of '-device'; Assign
PCI address for the device; Build
args of '-chardev')
* tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml: (See below)
* tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args: (See below)
* tests/qemuxml2argvtest.c: (Add tests)



---
  src/qemu/qemu_command.c  |   85 ++
  tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args |7 ++
  tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml  |   33 +
  tests/qemuxml2argvtest.c |2 +
  4 files changed, 127 insertions(+), 0 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml


No changes to SELinux ?  I'm reasonably sure the policy should
forbid QEMU from creating the shared memory backing file. For
hugepages, we had to create a per-QEMU directory, label that and
then pass it to QEMU.


Ah, I didn't realize that, especially when the selinux is disabled,
thanks for pointing it out.



This perhaps implies that our XML should not contain the actual
path. Libvirt can just create a path based on the name of the
device and set a label of the dir as for hugepages.


The shared memory server socket path is not created by QEMU, but
by a external application (called ivshmem_server [1]), creating
it in libvirt implies a force for users. They have to known our
rule first, and start the shared memory server with the right
socket path, and then starts the domain, generally it should be
opposite with the workflow which users want. So IMO it's not
a good way to go.

[1]: http://gitorious.org/nahanni/guest-code/trees/master/ivshmem-server

Regards,
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Eric Blake]

On 11/08/2012 07:24 AM, Daniel P. Berrange wrote:
 On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
 the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
 'start', 'stop', 'restart'; might be easier to remember than the
 current mix of 'start', 'shutdown', 'reboot'.

 * tools/virsh-domain.c (domManagementCmds): Add other command names.
 * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
 ---

 This patch documents both spellings.  An alternative would be to
 leave the alternate spellings as hidden aliases (virsh has support
 for that), but still mention them in virsh.pod (see how we did an
 alias for nodedev-dettach, for reference).
 
 NACK to this patch. I think the current command names are good.
 Creating duplicates will make life worse. First, it creates
 divergance from the similarly named commands for networks,
 storage and other objects. It also means scripts written again
 the new commands will not work with existing libvirt.

The patch is already in, but we also took care to explicitly document
that the new aliases are just that, and that portable scripts should use
the old name.

I don't see how this is any different to having both 'quit' and 'exit'
as aliases, nor even from 'detach-device' being documented as having an
older misspelled 'dettach-device' counterpart.  In other words, I don't
see that adding aliases is a problem.  On the other hand, I do agree
that we cannot remove any alias, once released, so we need to resolve
this situation prior to the release of 1.0.1.

 
 I actually think that shutdown  reboot are *better* names
 than restart and stop.
 
 If we wanted to replace any existing names, then the 'create'
 and 'destroy' names are the ones to replace, and for those I
 would expect to use 'boot' and 'stop'. I still don't thin
 we should do that either, due to creating inconsistency with
 other commands.

Although I have argued that aliases are helpful, I can understand your
counter-argument that too many aliases, or aliases that are unrelated to
the task at hand, are not wise.  If you would like to propose a
counter-patch that removes the just-added 'restart' alias, and
repurposes 'stop' to be an alias to 'destroy' rather than 'shutdown',
then that would leave us with:

'start' has alias 'boot'
'reboot' has no alias
'shutdown' has no alias
'destroy' has alias 'stop'

and I would feel comfortable ACK'ing such a patch before 1.0.1.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/4] docs: Add documents for memory device

[Osier Yang]

On 2012年11月08日 22:16, Daniel P. Berrange wrote:

On Wed, Nov 07, 2012 at 05:40:41PM +0800, Osier Yang wrote:

Considering there could be other memory related devices in future,
this introduces a general device model, called memory device,
instead of a specific device like ivshmem, currently only
ivshmem is supported though.

An example of the XML:

   memory model='ivshmem'


Since we have many sub-elements, I think it is preferrable to
also have the model in a subelement as we do with things like
disk/network/etc


Agreed.




 source id='nahanni' path='/tmp/nahanni'/


Use '/dev/shm' as the example path since that's more usual


No, the path is created by ivshmem_server, as I replied in
4/4.




 size unit='M'512/size
 vectors8/vectors
 /ioeventfd


Typo with the '/' location


Okay. :-)

Regards,
Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 10:51:14AM -0500, Dave Allan wrote:
 On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
  On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
   https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
   the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
   'start', 'stop', 'restart'; might be easier to remember than the
   current mix of 'start', 'shutdown', 'reboot'.
   
   * tools/virsh-domain.c (domManagementCmds): Add other command names.
   * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
   ---
   
   This patch documents both spellings.  An alternative would be to
   leave the alternate spellings as hidden aliases (virsh has support
   for that), but still mention them in virsh.pod (see how we did an
   alias for nodedev-dettach, for reference).
  
  NACK to this patch. I think the current command names are good.
  Creating duplicates will make life worse. First, it creates
  divergance from the similarly named commands for networks,
  storage and other objects. It also means scripts written again
  the new commands will not work with existing libvirt.
  
  I actually think that shutdown  reboot are *better* names
  than restart and stop.
  
  If we wanted to replace any existing names, then the 'create'
  and 'destroy' names are the ones to replace, and for those I
  would expect to use 'boot' and 'stop'. I still don't thin
  we should do that either, due to creating inconsistency with
  other commands.
 
 IMO this is fundamentally an aesthetic question here that's not going
 to be solved by debate, however, from the comments so far, the patch
 seems to have a fair amount of support.  DV, perhaps you could weigh
 in with your opinion?

Since this is an aesthetic question, even adding this patch is not going
to solve it. It is a fact of life that there are a huge variety of names
that could be used - adding more  more aliases is not a solution. You
can never satisfy everyone's desired name choice. You just have to pick
one name and stick with it consistently.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 10:30:55AM -0500, James B. Byrne wrote:
 
 On Thu, November 8, 2012 09:24, Daniel P. Berrange wrote:
 
 
  NACK to this patch. I think the current command names are good.
  Creating duplicates will make life worse. First, it creates
  divergance from the similarly named commands for networks,
  storage and other objects. It also means scripts written again
  the new commands will not work with existing libvirt.
 
 Duplicates (aliases) will make life worse for whom?  In what way?  On
 what evidence?  The present nomenclature is idiomatic to virsh and is
 a variance with how many people think of managing a host whether
 virtual or not.  On the other hand, one does not customarily speak of
 rebooting a network or a storage array, at least not in my experience.
 
 
  I actually think that shutdown  reboot are *better* names
  than restart and stop.
 
 Then change start to boot and be done with it.  But, the issue really
 is what English words are commonly associated with each other in the
 context we are dealing with.  I submit that 'start' is not intuitively
 associated with 'shutdown' by the vast majority of English speakers
 and hardly associated with 'reboot' by any.
 
 Consider the syntax of 'initctl' and 'service'.  Initctl uses start,
 stop and restart.  Service scripts virtually without exception use
 start, stop and restart, including that for libvirtd.  Operators are
 far more likely to be familiar with this combination of terms than any
 other.  Why force them to learn yet one more variant?  What is the
 advantage for the users?

If you are comparing to init services, then the 'stop' verb is semanticaly
equivalent to libvirt's 'destory' verb, not the shutdown verb. 'stop'
does a synchronous termination of the service.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 08:55:27AM -0700, Eric Blake wrote:
 On 11/08/2012 07:24 AM, Daniel P. Berrange wrote:
  On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
  the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
  'start', 'stop', 'restart'; might be easier to remember than the
  current mix of 'start', 'shutdown', 'reboot'.
 
  * tools/virsh-domain.c (domManagementCmds): Add other command names.
  * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
  ---
 
  This patch documents both spellings.  An alternative would be to
  leave the alternate spellings as hidden aliases (virsh has support
  for that), but still mention them in virsh.pod (see how we did an
  alias for nodedev-dettach, for reference).
  
  NACK to this patch. I think the current command names are good.
  Creating duplicates will make life worse. First, it creates
  divergance from the similarly named commands for networks,
  storage and other objects. It also means scripts written again
  the new commands will not work with existing libvirt.
 
 The patch is already in, but we also took care to explicitly document
 that the new aliases are just that, and that portable scripts should use
 the old name.

I really want to see that patch reverted from GIT.

 I don't see how this is any different to having both 'quit' and 'exit'
 as aliases, nor even from 'detach-device' being documented as having an
 older misspelled 'dettach-device' counterpart.  In other words, I don't
 see that adding aliases is a problem.  On the other hand, I do agree
 that we cannot remove any alias, once released, so we need to resolve
 this situation prior to the release of 1.0.1.

FWIW, I was not in favour of aliases in the first place, but I
defer to others in the case of previous aliases.

In this case though, I am really against the new proposed names
since I think 'stop' and 'restart' and a bad choice of names
as compared to our existing 'shutdown' and 'reboot'.

  I actually think that shutdown  reboot are *better* names
  than restart and stop.
  
  If we wanted to replace any existing names, then the 'create'
  and 'destroy' names are the ones to replace, and for those I
  would expect to use 'boot' and 'stop'. I still don't thin
  we should do that either, due to creating inconsistency with
  other commands.
 
 Although I have argued that aliases are helpful, I can understand your
 counter-argument that too many aliases, or aliases that are unrelated to
 the task at hand, are not wise.  If you would like to propose a
 counter-patch that removes the just-added 'restart' alias, and
 repurposes 'stop' to be an alias to 'destroy' rather than 'shutdown',
 then that would leave us with:
 
 'start' has alias 'boot'
 'reboot' has no alias
 'shutdown' has no alias
 'destroy' has alias 'stop'
 
 and I would feel comfortable ACK'ing such a patch before 1.0.1.

If we add an alias for 'destroy' we must do so for all instances
of 'destroy', including for storage, network.

I'm not really in favour of using 'boot' as an alias, since I
don't think 'boot' is a good term in the context of non-machine
based virtualization.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] AbortJob: Fix documentation

[Eric Blake]

On 11/08/2012 07:14 AM, Michal Privoznik wrote:
 This API was never synchronous and probably doesn't even need to be.
 ---
 
 I am sending this as a separate patch as regardless if my
 previous patch got accepted or not this one is still true.

ACK.

Note however that virDomainBlockJobAbort() defaults to synchronous; on
the other hand, that function also has a flags argument that can be used
to request async; too bad we don't have a flags argument for
virDomainAbortJob.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] storage: allow metadata preallocation when creating qcow2 images

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 04:24:51PM +0100, Ján Tomko wrote:
 Currently the 'allocation' element is not used when creating new images
 with qemu-img. This patch interprets a non-zero value as a request to
 preallocate metadata when a qcow2 image is created.
 
 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=684793
 ---
  src/storage/storage_backend.c |   19 +--
  1 files changed, 13 insertions(+), 6 deletions(-)
 
 diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
 index 41a19a1..bf6f7cc 100644
 --- a/src/storage/storage_backend.c
 +++ b/src/storage/storage_backend.c
 @@ -669,10 +669,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
  int imgformat = -1;
  virCommandPtr cmd = NULL;
  bool do_encryption = (vol-target.encryption != NULL);
 +bool preallocate = false;
  unsigned long long int size_arg;
  
  virCheckFlags(0, -1);
  
 +/* qcow2: preallocate metadata if requested allocation is non-zero */
 +if (vol-allocation  0  vol-target.format == VIR_STORAGE_FILE_QCOW2)
 +preallocate = true;
 +
  const char *type = virStorageFileFormatTypeToString(vol-target.format);
  const char *backingType = vol-backingStore.path ?
  virStorageFileFormatTypeToString(vol-backingStore.format) : NULL; 
 @@ -842,12 +847,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
   vol-target.path, NULL);
  virCommandAddArgFormat(cmd, %lluK, size_arg);
  
 -if (do_encryption) {
 -if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
 -virCommandAddArgList(cmd, -o, encryption=on, NULL);
 -} else {
 -virCommandAddArg(cmd, -e);
 -}
 +if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS 
 +(do_encryption || preallocate)) {
 +virCommandAddArg(cmd, -o);
 +virCommandAddArgFormat(cmd, %s%s%s, do_encryption ? 
 encryption=on : ,
 +   (do_encryption  preallocate) ? , : ,
 +   preallocate ? preallocation=metadata : 
 );
 +} else if (do_encryption){
 +virCommandAddArg(cmd, -e);
  }
  }

I've thought about doing it this way before, but I always felt it was
a bit of a gross hack. It would be nice to keep the 'allocation' element
as purely describing the data sector allocation. I was recently wondering
if we should just add a flag to the virStorageVolCreate like

  VIR_STORAGE_VOL_PREALLOC_METADATA


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Fix domain ID numbering race condition

[Eric Blake]

On 11/08/2012 06:09 AM, Peter Krempa wrote:
 When the libvirt daemon is restarted it tries to reconnect to running
 qemu domains. Since commit d38897a5d4b1880e1998394b2a37bba979bbdff1 the
 re-connection code runs in separate threads. In the original
 implementation the maximum of domain ID's (that is used as an
 initializer for numbering guests created next) while libvirt was
 reconnecting to the guest.
 

 +/* find the maximum ID from active and transient configs to initialize
 + * the driver with. This is to avoid race between autostart and reconnect
 + * threads */
 +virHashForEach(qemu_driver-domains.objs,
 +   qemuDomainFindMaxID,
 +   (qemu_driver-nextvmid));

Spurious parens; this can be just qemu_driver-nextvmid.

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Laine Stump]

On 11/08/2012 11:24 AM, Daniel P. Berrange wrote:
 On Thu, Nov 08, 2012 at 10:51:14AM -0500, Dave Allan wrote:
 On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
 On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
 the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
 'start', 'stop', 'restart'; might be easier to remember than the
 current mix of 'start', 'shutdown', 'reboot'.

 * tools/virsh-domain.c (domManagementCmds): Add other command names.
 * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
 ---

 This patch documents both spellings.  An alternative would be to
 leave the alternate spellings as hidden aliases (virsh has support
 for that), but still mention them in virsh.pod (see how we did an
 alias for nodedev-dettach, for reference).
 NACK to this patch. I think the current command names are good.
 Creating duplicates will make life worse. First, it creates
 divergance from the similarly named commands for networks,
 storage and other objects. It also means scripts written again
 the new commands will not work with existing libvirt.

 I actually think that shutdown  reboot are *better* names
 than restart and stop.

 If we wanted to replace any existing names, then the 'create'
 and 'destroy' names are the ones to replace, and for those I
 would expect to use 'boot' and 'stop'. I still don't thin
 we should do that either, due to creating inconsistency with
 other commands.
 IMO this is fundamentally an aesthetic question here that's not going
 to be solved by debate, however, from the comments so far, the patch
 seems to have a fair amount of support.  DV, perhaps you could weigh
 in with your opinion?
 Since this is an aesthetic question, even adding this patch is not going
 to solve it. It is a fact of life that there are a huge variety of names
 that could be used - adding more  more aliases is not a solution. You
 can never satisfy everyone's desired name choice. You just have to pick
 one name and stick with it consistently.

I'm putting my vote with Dan on this (Just to be clear, my original
message in this thread was written in the spirit of well, if you're
going to do this, then at least make sure what you're doing is
explicitly documented).

It's often a painful exercise to try and come up with the best name
for any particular operation, but once you've got a name, you've got a
name. After that, the truths are:

1) Multiple names for the same thing leads to confusion (which is what
prompted my earlier comment saying that the new names needed to be
clearly marked as synonyms).

2) There is no end to the new names that could be thought of as better
when taken in some particular context, or by some particular person.

3) If some action is named foo-x for foo objects, then a similar
action on fi objects should be named fi-x if at all possible. Maybe
x isn't the most appropriate name in the context of foo or fi, but
following a single model makes it easier to remember.

I know virsh isn't considered to be as strict of an environment as the
libvirt API proper, but we still need to be mindful of both backward
compatibility and of namespace pollution leading to confusion.

(If the current names bother you, you could just think of it from the
point of view of someone who has absolutely no knowledge of English, and
sees the command names as opaque cookies that produce the desired result.)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] net: update default.xml to match creation by libvirt

[Eric Blake]

I noticed that after a fresh install, the file
/etc/libvirt/qemu/networks/default.xml lacked the usual header
inserted by modern 'virsh net-edit'; and traced it to the fact
that libvirt.spec installs this file by copying a template
rather than using libvirt API.  We might as well make our template
match what libvirt itself would generate in that location.

* src/network/default.xml: Add header and newer XML details.
---
 src/network/default.xml | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/network/default.xml b/src/network/default.xml
index 9cfc01e..e124621 100644
--- a/src/network/default.xml
+++ b/src/network/default.xml
@@ -1,7 +1,14 @@
+!--
+WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
+OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
+  virsh net-edit default
+or other application using the libvirt API.
+--
+
 network
   namedefault/name
-  bridge name=virbr0 /
-  forward/
+  forward mode='nat'/
+  bridge name=virbr0 stp='on' delay='0'/
   ip address=192.168.122.1 netmask=255.255.255.0
 dhcp
   range start=192.168.122.2 end=192.168.122.254 /
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] v1: get, parse, and save dnsmasq version id

[Gene Czarcinski]

Use dnsmasq --version to obtain dnsmasq's version id and
save the major/minor values.  This is a separate patch since other
patches will depend on it.
---
 src/conf/network_conf.h |  2 ++
 src/network/bridge_driver.c | 35 +--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3e46304..cfc49af 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -218,6 +218,8 @@ struct _virNetworkObj {
 unsigned int active : 1;
 unsigned int autostart : 1;
 unsigned int persistent : 1;
+unsigned int dnsmasqMajor;
+unsigned int dnsmasqMinor;
 
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0c4c794..19610f2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -968,21 +968,51 @@ cleanup:
 return ret;
 }
 
+#define DNSMASQ_VERSION_PREFIX Dnsmasq version 
+
 static int
 networkStartDhcpDaemon(virNetworkObjPtr network)
 {
 virCommandPtr cmd = NULL;
+const char *cmdname = DNSMASQ;
+char *tmp, *version = NULL;
+unsigned int  major = 0, minor = 0;
 char *pidfile = NULL;
 char *testconfigstr = NULL;
 int ret = -1;
 dnsmasqContext *dctx = NULL;
 
 if (!virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, 0)) {
-/* no IPv6 addresses, so we don't need to run radvd */
+/* no IP addresses, so we don't need to run */
 ret = 0;
 goto cleanup;
 }
-VIR_INFO(starting dhcp daemon (dnsmasq));
+
+if (!virFileIsExecutable(cmdname)) {
+VIR_WARN(file %s missing or not executable, cmdname);
+goto cleanup;
+}
+VIR_INFO(starting dhcp daemon (%s), cmdname);
+
+cmd = virCommandNew(cmdname);
+virCommandAddArg(cmd, --version);
+virCommandSetOutputBuffer(cmd, version);
+if (virCommandRun(cmd, NULL)  0)
+goto cleanup;
+virCommandFree(cmd);
+cmd = NULL;
+
+if ((version!=NULL) 
+(strncmp(version, DNSMASQ_VERSION_PREFIX, 
strlen(DNSMASQ_VERSION_PREFIX))==0)) {
+tmp = version + strlen(DNSMASQ_VERSION_PREFIX);
+if (virStrToLong_ui(tmp, tmp, 10, major) = 0) {
+if ((*tmp == '.') 
+virStrToLong_ui(tmp + 1, tmp, 10, minor) = 0) {
+network-dnsmasqMajor = major;
+network-dnsmasqMinor = minor;
+}
+}
+}
 
 if (virFileMakePath(NETWORK_PID_DIR)  0) {
 virReportSystemError(errno,
@@ -1043,6 +1073,7 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
 ret = 0;
 cleanup:
 VIR_FREE(pidfile);
+VIR_FREE(version);
 virCommandFree(cmd);
 dnsmasqContextFree(dctx);
 return ret;
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] v1: use dnsmasq instead of radvd to handle RA service

[Gene Czarcinski]

This patch tests the version of dnsmasq.  If (major  2)
or ((major==2) and (minor = 63)), then use dnsmasq.  Otherwise,
use radvd to provide the RA service.
---
 src/conf/network_conf.h|  1 +
 src/network/bridge_driver.c| 36 ++
 tests/networkxml2argvdata/dhcp6-network.argv   |  1 +
 tests/networkxml2argvdata/nat-network-dhcp6.argv   |  1 +
 .../nat-network-dns-srv-record-minimal.argv|  1 +
 .../nat-network-dns-srv-record.argv|  1 +
 .../nat-network-dns-txt-record.argv|  1 +
 tests/networkxml2argvdata/nat-network.argv |  1 +
 .../routed-network-dhcphost.argv   |  1 +
 tests/networkxml2argvtest.c|  1 +
 10 files changed, 45 insertions(+)

diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index cfc49af..39fffd7 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -220,6 +220,7 @@ struct _virNetworkObj {
 unsigned int persistent : 1;
 unsigned int dnsmasqMajor;
 unsigned int dnsmasqMinor;
+unsigned int dnsmasqRA;
 
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 19610f2..2230268 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -893,6 +893,26 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 virBufferAsprintf(configbuf, addn-hosts=%s\n,
  dctx-addnhostsfile-path);
 
+/* we are doing RA instead of radvd */
+if (network-dnsmasqRA) {
+if (dhcp6flag)
+virBufferAsprintf(configbuf, enable-ra\n);
+else {
+char *bridgeaddr = NULL;
+ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, 0);
+if (ipdef) {
+bridgeaddr = virSocketAddrFormat(ipdef-address);
+if (bridgeaddr) {
+virBufferAsprintf(configbuf,
+dhcp-range=%s,ra-only\n, bridgeaddr);
+}
+}
+if ((!ipdef) || (!bridgeaddr))
+ VIR_WARN(IPv6 invalid configuration for %s, 
network-def-name);
+VIR_FREE(bridgeaddr);
+}
+}
+
 if (!(*configstr = virBufferContentAndReset(configbuf))) {
 virReportOOMError();
 goto cleanup;
@@ -1014,6 +1034,12 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
 }
 }
 
+/* is radvd or dnsmasq to handle RA? */
+if ((network-dnsmasqMajor  2) ||
+((network-dnsmasqMajor == 2) 
+ (network-dnsmasqMinor = 63)))
+network-dnsmasqRA = 1;
+
 if (virFileMakePath(NETWORK_PID_DIR)  0) {
 virReportSystemError(errno,
  _(cannot create directory %s),
@@ -1293,6 +1319,12 @@ networkStartRadvd(virNetworkObjPtr network)
 virCommandPtr cmd = NULL;
 int ret = -1;
 
+/* is dnsmasq handling RA */
+if (network-dnsmasqRA) {
+ret = 0;
+goto cleanup;
+}
+
 network-radvdPid = -1;
 
 if (!virNetworkDefGetIpByIndex(network-def, AF_INET6, 0)) {
@@ -1370,6 +1402,10 @@ cleanup:
 static int
 networkRefreshRadvd(virNetworkObjPtr network)
 {
+/* is dnsmasq handling RA */
+if (network-dnsmasqRA)
+return 0;
+
 /* if there's no running radvd, just start it */
 if (network-radvdPid = 0 || (kill(network-radvdPid, 0)  0))
 return networkStartRadvd(network);
diff --git a/tests/networkxml2argvdata/dhcp6-network.argv 
b/tests/networkxml2argvdata/dhcp6-network.argv
index 6697833..66ec430 100644
--- a/tests/networkxml2argvdata/dhcp6-network.argv
+++ b/tests/networkxml2argvdata/dhcp6-network.argv
@@ -14,3 +14,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=240
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
+enable-ra
diff --git a/tests/networkxml2argvdata/nat-network-dhcp6.argv 
b/tests/networkxml2argvdata/nat-network-dhcp6.argv
index 14256ae..c56fff1 100644
--- a/tests/networkxml2argvdata/nat-network-dhcp6.argv
+++ b/tests/networkxml2argvdata/nat-network-dhcp6.argv
@@ -17,3 +17,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=493
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
+enable-ra
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv 
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
index 45a658b..26f283e 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -17,3 +17,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=253
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Laine Stump]

On 11/07/2012 04:16 PM, Eric Blake wrote:
 On 11/07/2012 02:05 PM, Gene Czarcinski wrote:
 As I was doing this, I also looked through the libvirt.spec file. My,
 what a wonderful example of wizardly that is.  Anyway, I thought some
 updates may be in ortder:

 - increase the minimum version for dnsmasq from 2.41 to 2.48.
 That's probably safe, since it still works for RHEL 5.9.

Actually, RHEL 5.9 (and therefore CentOS 5) is stuck at dnsmasq-2.45,
and there's very little chance it will ever be rebased again.

So if the minimum version is going to be increased, it should be done
conditionally according to the release version.


 - why is radvd required for rpmbuild?
 So that ./configure finds the radvd binaries, and encodes the correct
 location into the built libvirt, for the case where libvirt will be
 using radvd.  The dependency makes sense for distros where we plan on
 using radvd, but can be made conditional so that we skip it on distros
 where using dnsmasq alone is sufficient.

BTW, we'll want to do some reasonable testing of this replacement before
foisting it on everyone.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] net: update default.xml to match creation by libvirt

[Laine Stump]

On 11/08/2012 01:06 PM, Eric Blake wrote:
 I noticed that after a fresh install, the file
 /etc/libvirt/qemu/networks/default.xml lacked the usual header
 inserted by modern 'virsh net-edit'; and traced it to the fact
 that libvirt.spec installs this file by copying a template
 rather than using libvirt API.  We might as well make our template
 match what libvirt itself would generate in that location.

 * src/network/default.xml: Add header and newer XML details.
 ---
  src/network/default.xml | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 diff --git a/src/network/default.xml b/src/network/default.xml
 index 9cfc01e..e124621 100644
 --- a/src/network/default.xml
 +++ b/src/network/default.xml
 @@ -1,7 +1,14 @@
 +!--
 +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
 +OVERWRITTEN AND LOST. Changes to this xml configuration should be made using:
 +  virsh net-edit default
 +or other application using the libvirt API.
 +--
 +
  network
namedefault/name
 -  bridge name=virbr0 /
 -  forward/
 +  forward mode='nat'/

mode='nat' is the defined default, so that isn't strictly necessary...

 +  bridge name=virbr0 stp='on' delay='0'/

The above line shouldn't be in the file. This will create conflicts if
someone has a libvirt installation that already has networks defined
(and so is already using virbr0 for a different network), and you go
through the back door to create a network definition that re-uses it.
(which makes me wonder if the auto-created bridge name used by a default
network created in this manner will end up being written to the config
file in /etc...)

ip address=192.168.122.1 netmask=255.255.255.0
  dhcp
range start=192.168.122.2 end=192.168.122.254 /


Actually there is another open bug about the way we're creating this
network:

  https://bugzilla.redhat.com/show_bug.cgi?id=867546

The problem is that when libvirtd is already running and you install the
libvirt-daemon-config-network-config package (which I believe is what
happens during the initial install of everything), the default network
doesn't show up in libvirt's list until the next time you restart
libvirtd (because we've just copied it into place on the disk, so
libvirtd hasn't been told about it).


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Eric Blake]

On 11/08/2012 08:26 AM, Doug Goldstein wrote:
 I'm still not thrilled that you're pushing forward with requiring 2.63
 + a few patches backported from 2.64 into 2.63 and only checking
 against 2.63.


 My point is if you're going to add a check for 2.63
 but really require 2.63 + 3 patches that Fedora has backported into
 their 2.63 version which was your original proposal, this would cause
 lots of headaches for every other distro out there unless they
 backported those very same patches into 2.63. So better to wait for
 2.64 and go forward from there. libvirt works on and targets many more
 systems than Fedora.

Agreed.  Upstream, libvirt should require 2.64.  If Fedora (or any other
distro) cares about shipping 2.63 + patches, then they can also patch
their backport of libvirt to relax things to 2.63.  But upstream cannot
assume that 2.63 is patched.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] net: update default.xml to match creation by libvirt

[Laine Stump]

On 11/08/2012 01:51 PM, Laine Stump wrote:
 On 11/08/2012 01:06 PM, Eric Blake wrote:
 I noticed that after a fresh install, the file
 /etc/libvirt/qemu/networks/default.xml lacked the usual header
 inserted by modern 'virsh net-edit'; and traced it to the fact
 that libvirt.spec installs this file by copying a template
 rather than using libvirt API.  We might as well make our template
 match what libvirt itself would generate in that location.

 * src/network/default.xml: Add header and newer XML details.
 ---
  src/network/default.xml | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 diff --git a/src/network/default.xml b/src/network/default.xml
 index 9cfc01e..e124621 100644
 --- a/src/network/default.xml
 +++ b/src/network/default.xml
 @@ -1,7 +1,14 @@
 +!--
 +WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
 +OVERWRITTEN AND LOST. Changes to this xml configuration should be made 
 using:
 +  virsh net-edit default
 +or other application using the libvirt API.
 +--
 +
  network
namedefault/name
 -  bridge name=virbr0 /
 -  forward/
 +  forward mode='nat'/
 mode='nat' is the defined default, so that isn't strictly necessary...

 +  bridge name=virbr0 stp='on' delay='0'/
 The above line shouldn't be in the file. This will create conflicts if
 someone has a libvirt installation that already has networks defined
 (and so is already using virbr0 for a different network), and you go
 through the back door to create a network definition that re-uses it.
 (which makes me wonder if the auto-created bridge name used by a default
 network created in this manner will end up being written to the config
 file in /etc...)

I just checked this out and the results were actually a bit disturbing:

As you'd expect, if you blindly put bridge name='virbr0'/ in the file
when an existing network already uses virbr0, then one of the two
networks will fail to start (depending on which is started first.

HOWEVER, if you don't put in any bridge at all, net-start will take
the lack of a bridge device to mean that you actually want virbr0, so
the results are identical :-(

Additionally, although forward/ is always formatted as forward
mode='nat'/ for net-dumpxml, it isn't actually replaced in the
persistent config.

So, there are really 3(!) bugs:

1) the bug in the BZ
2) if you don't specify a bridge device to use, it will always try to
use virbr0 rather than looking for one that isn't in use,
3) The choice of bridge devices is never updated in the persistent config.

One way to solve all of them would be to use virsh define default.xml
 virsh autostart default  virsh start default rather than directly
inserting the file, but we would need to be certain that libvirtd was
always running by the time the %post script for
libvirt-daemon-config-network was run (and that may be the case, I
haven't looked). In that case, your patch to change the .xml file on
disk wouldn't be necessary.


ip address=192.168.122.1 netmask=255.255.255.0
  dhcp
range start=192.168.122.2 end=192.168.122.254 /

 Actually there is another open bug about the way we're creating this
 network:

   https://bugzilla.redhat.com/show_bug.cgi?id=867546

 The problem is that when libvirtd is already running and you install the
 libvirt-daemon-config-network-config package (which I believe is what
 happens during the initial install of everything), the default network
 doesn't show up in libvirt's list until the next time you restart
 libvirtd (because we've just copied it into place on the disk, so
 libvirtd hasn't been told about it).


 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Laine Stump]

On 11/08/2012 01:55 PM, Eric Blake wrote:
 On 11/08/2012 08:26 AM, Doug Goldstein wrote:
 I'm still not thrilled that you're pushing forward with requiring 2.63
 + a few patches backported from 2.64 into 2.63 and only checking
 against 2.63.
 My point is if you're going to add a check for 2.63
 but really require 2.63 + 3 patches that Fedora has backported into
 their 2.63 version which was your original proposal, this would cause
 lots of headaches for every other distro out there unless they
 backported those very same patches into 2.63. So better to wait for
 2.64 and go forward from there. libvirt works on and targets many more
 systems than Fedora.
 Agreed.  Upstream, libvirt should require 2.64.  If Fedora (or any other
 distro) cares about shipping 2.63 + patches, then they can also patch
 their backport of libvirt to relax things to 2.63.  But upstream cannot
 assume that 2.63 is patched.

Really I don't think that *anybody* should. There's no way to verify
that it's true.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Gene Czarcinski]

On 11/08/2012 01:55 PM, Eric Blake wrote:

On 11/08/2012 08:26 AM, Doug Goldstein wrote:

I'm still not thrilled that you're pushing forward with requiring 2.63
+ a few patches backported from 2.64 into 2.63 and only checking
against 2.63.

My point is if you're going to add a check for 2.63
but really require 2.63 + 3 patches that Fedora has backported into
their 2.63 version which was your original proposal, this would cause
lots of headaches for every other distro out there unless they
backported those very same patches into 2.63. So better to wait for
2.64 and go forward from there. libvirt works on and targets many more
systems than Fedora.

Agreed.  Upstream, libvirt should require 2.64.  If Fedora (or any other
distro) cares about shipping 2.63 + patches, then they can also patch
their backport of libvirt to relax things to 2.63.  But upstream cannot
assume that 2.63 is patched.


Understood ... I think.

1.  For RADVD, if dnsmasq version = 2.63, then dnsmasq will handle the 
RA service.  Otherwise, it will be radvd.  My submitted patches 
implement that.  Granted, the patches are, more or less, chained 
together and that will make life difficult.


2.  Currently, there are NO version checks in the DHCP6 implementation 
(all DHCPv6 stuff is in one patch file currently v6-7).  If I understand 
you correctly, you want dnsmasq version checks performed.  Some of this 
is simple and some is not so simple.


2a. If the dsnmasq version is = 2.64, all the DHCPv6 stuff is OK.  This 
check is fairly easy to do and I will try to get an implementation in 
today so that things might move along.  I would like to see this as a 
minimum since I had modified and added to the tests performed.  Pulling 
a piece out of the middle will be difficult.


2b. If dnsmasq version is =2.63, then ??

  -- fail/error out the dnsmasq startup because the xml specification 
is invalid.


  -- or, just ignore the any DHCPv6 dhcp-range or dhcp-host 
specifications but issue a syslog warning message that it is being 
ignored because of the dnsmasq version.


  -- or ? ... any suggestions?

3.  I would like to make it easy for any system administratiors or 
system maintainers who want DHCPv6 but also need to keep dnsmasq 2.63.  
It it is too difficult, they will simply use dnsmasq 2.64test or 2.64rc 
or 2.64 ... unlike libvirt, it only takes about 15 seconds to rebuild 
the dnsmasq rpms.  At some point in time in the not so distant future, 
this becomes a non-issue ... it is the transition period.


3a. Do nothing!  Screw it. If someone wants DHCPv6 support, then they 
can get some version of dnsmasq 2.64 since it will be here real soon 
now.  It has always been my contention that if someone wants libvirt 
with DHCPV6 support, they can got the little extra to get a dnsmasq that 
supports it.


3b. Add a configuration/compile-time options allowing 2.63.

Right now I am inclined to do 3a.  But, the question I have is what to 
do for 2b.  The simplest way (and this is my inclination for today) is 
to error out.


Right now I feel a little pressured because I am going to be unavailable 
for the next week to ten days.


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Laine Stump]

On 11/07/2012 04:25 PM, Gene Czarcinski wrote:
 On 11/06/2012 11:23 AM, Gene Czarcinski wrote:
 ip6tables -I FORWARD 1 -i  virbr18 -o  virbr18  -j  ACCEPT
 This one currently isn't getting added, because
 networkAddGeneralIp6tablesRules() returns immediately if there are no
 ipv6 addresses defined for the network.

 Note that up until now, unless someone had an ipv6 address defined
 for a
 network, ip6tables was never called, so theoretically you could run
 libvirtd without having ip6tables installed on your machine, but with
 this change *all* networks would fail to start if the ip6tables binary
 was missing. That *shouldn't* be a problem because (at least on
 Fedora/RHEL/CentOS) it is a part of the same package as iptables, but
 I've seen strange setups in the last few years - in particular I recall
 one Gentoo user whose networks were mysteriously failing, and it was
 because he had built the iproute package with some sort of minimal
 configuration that's available on Gentoo, causing something or other to
 fail in a mysterious way (compounded in troubleshooting by the fact
 that
 none of us would ever have expected such a thing).
 How about a configure/compile time option for those systems which may
 not have ip6tables ... the default being that ip6tables is assumed. 
 ping

Sorry, I've been overwhelmed and am churning.


 OK, I have not heard a plain yes or no on this.

 IPv4 and IPv6 networks are suppose to have the same (more or less)
 functionality so why isn't this OK.

Maintaining backward compatibility, both API and operational. In the
past it wasn't the case that we simply did nothing about ipv6 on
libvirt's networks, instead we explicitly set a sysctl to *disable* it.
That must have been done for some reason. That reason may no longer be
valid, but we don't know that yet (it happened before I was around). If
the reason is no longer valid, we can go ahead as you suggest (and I
would say we don't even need an option to not have ip6tables, just force
people to build the full iptables package as God intended :-P). If the
reason *is* still valid, then we need to only enable the ipv6 sysctl and
add the ip6tables rule in response to some new flag attribute in the
network config.

 If you do want to give someone the option of running without
 ip6tables, fine make it a compile-time option.

Actually I want to hear some historical info about why ipv6 was
explicitly *disabled* with sysctl on libvirt's networks in the past.
Maybe it's time to change that default, but I don't want to do it
lightly - it may even have been done in response to a CVE. (danpb or DV
would probably have the best insight about this. I'll point them at this
thread.)

New functionality is great, but you can't upset working systems. (I know
I'm very tedious and am sounding like a broken record, but this is a
topic that libvirt takes *very* seriously; the API must always be 100%
backward compatible, and consciously programmed behavior should always
remain the same.)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Laine Stump]

On 11/08/2012 02:30 PM, Gene Czarcinski wrote:
 On 11/08/2012 01:55 PM, Eric Blake wrote:
 On 11/08/2012 08:26 AM, Doug Goldstein wrote:
 I'm still not thrilled that you're pushing forward with requiring
 2.63
 + a few patches backported from 2.64 into 2.63 and only checking
 against 2.63.
 My point is if you're going to add a check for 2.63
 but really require 2.63 + 3 patches that Fedora has backported into
 their 2.63 version which was your original proposal, this would cause
 lots of headaches for every other distro out there unless they
 backported those very same patches into 2.63. So better to wait for
 2.64 and go forward from there. libvirt works on and targets many more
 systems than Fedora.
 Agreed.  Upstream, libvirt should require 2.64.  If Fedora (or any other
 distro) cares about shipping 2.63 + patches, then they can also patch
 their backport of libvirt to relax things to 2.63.  But upstream cannot
 assume that 2.63 is patched.

 Understood ... I think.

 1.  For RADVD, if dnsmasq version = 2.63, then dnsmasq will handle
 the RA service.  Otherwise, it will be radvd.  My submitted patches
 implement that.  Granted, the patches are, more or less, chained
 together and that will make life difficult.

 2.  Currently, there are NO version checks in the DHCP6 implementation
 (all DHCPv6 stuff is in one patch file currently v6-7).  If I
 understand you correctly, you want dnsmasq version checks performed. 
 Some of this is simple and some is not so simple.

 2a. If the dsnmasq version is = 2.64, all the DHCPv6 stuff is OK. 
 This check is fairly easy to do and I will try to get an
 implementation in today so that things might move along.  I would like
 to see this as a minimum since I had modified and added to the tests
 performed.  Pulling a piece out of the middle will be difficult.

 2b. If dnsmasq version is =2.63, then ??

   -- fail/error out the dnsmasq startup because the xml specification
 is invalid.

   -- or, just ignore the any DHCPv6 dhcp-range or dhcp-host
 specifications but issue a syslog warning message that it is being
 ignored because of the dnsmasq version.

Since they are explicitly asking for this functionality, it's better to
log an error and fail the network start. This will alert them that they
aren't getting what they asked for in a very prominent manner.



   -- or ? ... any suggestions?

 3.  I would like to make it easy for any system administratiors or
 system maintainers who want DHCPv6 but also need to keep dnsmasq
 2.63.  It it is too difficult, they will simply use dnsmasq 2.64test
 or 2.64rc or 2.64 ... unlike libvirt, it only takes about 15 seconds
 to rebuild the dnsmasq rpms.  At some point in time in the not so
 distant future, this becomes a non-issue ... it is the transition period.

 3a. Do nothing!  Screw it. If someone wants DHCPv6 support, then they
 can get some version of dnsmasq 2.64 since it will be here real soon
 now.  It has always been my contention that if someone wants libvirt
 with DHCPV6 support, they can got the little extra to get a dnsmasq
 that supports it.

 3b. Add a configuration/compile-time options allowing 2.63.

 Right now I am inclined to do 3a.

I agree. If you need 2.64 for proper DHCPv6 support, then just require
that. Anybody who can upgrade to 2.63 can upgrade to 2.64 (my guess is
that Fedora is unique in having 2.63 as its official version for any
release, and that's only for F17 (it seems reasonable that we could get
dnsmasq for F18 upgraded to 2.64, maybe even F17, with appropriate
testing (and depending on the adventurousness index of the new Fedora
dnsmasq maintainer :-)

   But, the question I have is what to do for 2b.  The simplest way
 (and this is my inclination for today) is to error out.

I agree with that as well.


 Right now I feel a little pressured because I am going to be
 unavailable for the next week to ten days.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Add NUMA memory and CPU thread siblings to capabilities

[Dusty Mabe]

This patch is a follow up to 
https://www.redhat.com/archives/libvir-list/2012-October/msg01382.html.
Since that one hasn't been reviewed yet I combined that work with new work to 
add memory
information to virsh capabilities output and am submitting them both in this 
patch.

With this virsh capabilities output will have the following form:

topology
  cells num='2'
cell id='0'
  memory unit='KiB'12572412/memory
  cpus num='12'
cpu id='0' thread_siblings='0,12'/
.
.
  /cpus
/cell
  /cells
/topology

Hope you guys can find this useful.

Dusty

Dusty Mabe (1):
  Add NUMA memory and CPU thread siblings to capabilities

 docs/schemas/capability.rng   |  15 +++
 src/conf/capabilities.c   |  65 ++---
 src/conf/capabilities.h   |   7 +-
 src/nodeinfo.c| 155 +-
 src/test/test_driver.c|   2 +-
 src/xen/xend_internal.c   |   4 +-
 tests/capabilityschemadata/caps-test3.xml |  88 +
 7 files changed, 319 insertions(+), 17 deletions(-)
 create mode 100644 tests/capabilityschemadata/caps-test3.xml

-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Add NUMA memory and CPU thread siblings to capabilities

[Dusty Mabe]

---
 docs/schemas/capability.rng   |  15 +++
 src/conf/capabilities.c   |  65 ++---
 src/conf/capabilities.h   |   7 +-
 src/nodeinfo.c| 155 +-
 src/test/test_driver.c|   2 +-
 src/xen/xend_internal.c   |   4 +-
 tests/capabilityschemadata/caps-test3.xml |  88 +
 7 files changed, 319 insertions(+), 17 deletions(-)
 create mode 100644 tests/capabilityschemadata/caps-test3.xml

diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 8c928bc..1ab36ed 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -176,6 +176,10 @@
   /attribute
 
   optional
+ref name='memory'/
+  /optional
+
+  optional
 element name='cpus'
   attribute name='num'
 ref name='unsignedInt'/
@@ -188,11 +192,22 @@
 /element
   /define
 
+  define name='memory'
+element name='memory'
+ref name='scaledInteger'/
+/element
+  /define
+
   define name='cpu'
 element name='cpu'
   attribute name='id'
 ref name='unsignedInt'/
   /attribute
+  optional
+attribute name='thread_siblings'
+  text/
+/attribute
+  /optional
 /element
   /define
 
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index a8ee2cf..1b514ef 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -73,10 +73,16 @@ virCapabilitiesNew(const char *arch,
 static void
 virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
 {
+int i;
 if (cell == NULL)
 return;
 
+if (cell-threadSiblings)
+for (i=0; i  cell-ncpus; i++)
+VIR_FREE(cell-threadSiblings[i]);
+
 VIR_FREE(cell-cpus);
+VIR_FREE(cell-threadSiblings);
 VIR_FREE(cell);
 }
 
@@ -253,7 +259,10 @@ int
 virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
-   const int *cpus)
+   unsigned long long mem,
+   const int *cpus,
+   const virBitmapPtr *threadSiblings
+)
 {
 virCapsHostNUMACellPtr cell;
 
@@ -272,8 +281,20 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
cpus,
ncpus * sizeof(*cpus));
 
+if (threadSiblings) {
+if (VIR_ALLOC_N(cell-threadSiblings, ncpus)  0) {
+VIR_FREE(cell-cpus);
+VIR_FREE(cell);
+return -1;
+}
+memcpy(cell-threadSiblings,
+   threadSiblings,
+   ncpus * sizeof(*threadSiblings));
+}
+
 cell-ncpus = ncpus;
 cell-num = num;
+cell-mem = mem;
 
 caps-host.numaCell[caps-host.nnumaCell++] = cell;
 
@@ -695,6 +716,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
 virBuffer xml = VIR_BUFFER_INITIALIZER;
 int i, j, k;
 char host_uuid[VIR_UUID_STRING_BUFLEN];
+char *str;
 
 virBufferAddLit(xml, capabilities\n\n);
 virBufferAddLit(xml,   host\n);
@@ -754,22 +776,43 @@ virCapabilitiesFormatXML(virCapsPtr caps)
 }
 
 if (caps-host.nnumaCell) {
-virBufferAddLit(xml, topology\n);
-virBufferAsprintf(xml,   cells num='%zu'\n,
+virBufferAdjustIndent(xml, 4);
+virBufferAddLit(xml, topology\n);
+virBufferAsprintf(xml,   cells num='%zu'\n,
   caps-host.nnumaCell);
 for (i = 0 ; i  caps-host.nnumaCell ; i++) {
-virBufferAsprintf(xml, cell id='%d'\n,
+virBufferAsprintf(xml, cell id='%d'\n,
   caps-host.numaCell[i]-num);
-virBufferAsprintf(xml,   cpus num='%d'\n,
+
+/* Print out the numacell memory total if it is available */
+if (caps-host.numaCell[i]-mem)
+virBufferAsprintf(xml,   memory 
unit='KiB'%llu/memory\n,
+  caps-host.numaCell[i]-mem);
+
+virBufferAsprintf(xml,   cpus num='%d'\n,
   caps-host.numaCell[i]-ncpus);
-for (j = 0 ; j  caps-host.numaCell[i]-ncpus ; j++)
-virBufferAsprintf(xml, cpu id='%d'/\n,
+for (j = 0 ; j  caps-host.numaCell[i]-ncpus ; j++) {
+virBufferAsprintf(xml, cpu id='%d',
   caps-host.numaCell[i]-cpus[j]);
-virBufferAddLit(xml,   /cpus\n);
-virBufferAddLit(xml, /cell\n);
+
+/* Print out thread siblings if they were populated */
+if (caps-host.numaCell[i]-threadSiblings) {
+str = 
virBitmapFormat(caps-host.numaCell[i]-threadSiblings[j]);
+if (str) {
+virBufferAsprintf(xml,  thread_siblings='%s', str);
+  

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Gene Czarcinski]

On 11/08/2012 02:59 PM, Laine Stump wrote:

On 11/08/2012 02:30 PM, Gene Czarcinski wrote:

On 11/08/2012 01:55 PM, Eric Blake wrote:

On 11/08/2012 08:26 AM, Doug Goldstein wrote:

I'm still not thrilled that you're pushing forward with requiring
2.63
+ a few patches backported from 2.64 into 2.63 and only checking
against 2.63.

My point is if you're going to add a check for 2.63
but really require 2.63 + 3 patches that Fedora has backported into
their 2.63 version which was your original proposal, this would cause
lots of headaches for every other distro out there unless they
backported those very same patches into 2.63. So better to wait for
2.64 and go forward from there. libvirt works on and targets many more
systems than Fedora.

Agreed.  Upstream, libvirt should require 2.64.  If Fedora (or any other
distro) cares about shipping 2.63 + patches, then they can also patch
their backport of libvirt to relax things to 2.63.  But upstream cannot
assume that 2.63 is patched.


Understood ... I think.

1.  For RADVD, if dnsmasq version = 2.63, then dnsmasq will handle
the RA service.  Otherwise, it will be radvd.  My submitted patches
implement that.  Granted, the patches are, more or less, chained
together and that will make life difficult.

2.  Currently, there are NO version checks in the DHCP6 implementation
(all DHCPv6 stuff is in one patch file currently v6-7).  If I
understand you correctly, you want dnsmasq version checks performed.
Some of this is simple and some is not so simple.

2a. If the dsnmasq version is = 2.64, all the DHCPv6 stuff is OK.
This check is fairly easy to do and I will try to get an
implementation in today so that things might move along.  I would like
to see this as a minimum since I had modified and added to the tests
performed.  Pulling a piece out of the middle will be difficult.

2b. If dnsmasq version is =2.63, then ??

   -- fail/error out the dnsmasq startup because the xml specification
is invalid.

   -- or, just ignore the any DHCPv6 dhcp-range or dhcp-host
specifications but issue a syslog warning message that it is being
ignored because of the dnsmasq version.

Since they are explicitly asking for this functionality, it's better to
log an error and fail the network start. This will alert them that they
aren't getting what they asked for in a very prominent manner.



   -- or ? ... any suggestions?

3.  I would like to make it easy for any system administratiors or
system maintainers who want DHCPv6 but also need to keep dnsmasq
2.63.  It it is too difficult, they will simply use dnsmasq 2.64test
or 2.64rc or 2.64 ... unlike libvirt, it only takes about 15 seconds
to rebuild the dnsmasq rpms.  At some point in time in the not so
distant future, this becomes a non-issue ... it is the transition period.

3a. Do nothing!  Screw it. If someone wants DHCPv6 support, then they
can get some version of dnsmasq 2.64 since it will be here real soon
now.  It has always been my contention that if someone wants libvirt
with DHCPV6 support, they can got the little extra to get a dnsmasq
that supports it.

3b. Add a configuration/compile-time options allowing 2.63.

Right now I am inclined to do 3a.

I agree. If you need 2.64 for proper DHCPv6 support, then just require
that. Anybody who can upgrade to 2.63 can upgrade to 2.64 (my guess is
that Fedora is unique in having 2.63 as its official version for any
release, and that's only for F17 (it seems reasonable that we could get
dnsmasq for F18 upgraded to 2.64, maybe even F17, with appropriate
testing (and depending on the adventurousness index of the new Fedora
dnsmasq maintainer :-)


OK, it is done ... now I just have to rework the submitted patches:

dnsmasq-RA -2.63, otherwise radvd.  DHCPv6 dnsmasq =2.64 and error out 
if not. .. it only took a few lines of code.



   But, the question I have is what to do for 2b.  The simplest way
(and this is my inclination for today) is to error out.

I agree with that as well.


Right now I feel a little pressured because I am going to be
unavailable for the next week to ten days.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list




--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/6] dnsmasq conf-file, DHCPv6, dnsmasq-RA updates

[Gene Czarcinski]

OK, here it is.  There are a related set of patches in that they are
mostly dependent and need to be applied in the order show..

The individual patch files explain what each one is.

Note that patch file 0003 for DHCPv6  adds some test files.

Note that patch file 0006 add a test for the dnsmasq version for
DHCPv6 support and if it is less that 2.64, it errors out.

Gene Czarcinski (6):
  v6-6: put dnsmasq parameters into a file
  v6-6: add dnsmasq interface= parameter
  v6-7: Add support for DHCPv6
  v1: get, parse, and save dnsmasq version id
  v1: use dnsmasq instead of radvd to handle RA service
  v1: for DHCPv6, add dnsmasq version check

 docs/formatnetwork.html.in | 108 +++-
 src/conf/network_conf.c| 100 ++--
 src/conf/network_conf.h|   3 +
 src/network/bridge_driver.c| 541 ++---
 src/network/bridge_driver.h|   7 +-
 src/util/dnsmasq.c |   9 +-
 tests/networkxml2argvdata/dhcp6-network.argv   |  17 +
 tests/networkxml2argvdata/dhcp6-network.xml|  16 +
 tests/networkxml2argvdata/isolated-network.argv|  25 +-
 tests/networkxml2argvdata/nat-network-dhcp6.argv   |  20 +
 tests/networkxml2argvdata/nat-network-dhcp6.xml|  26 +
 .../networkxml2argvdata/nat-network-dns-hosts.argv |  15 +-
 .../nat-network-dns-srv-record-minimal.argv|  37 +-
 .../nat-network-dns-srv-record.argv|  37 +-
 .../nat-network-dns-txt-record.argv|  31 +-
 tests/networkxml2argvdata/nat-network.argv |  29 +-
 tests/networkxml2argvdata/netboot-network.argv |  29 +-
 .../networkxml2argvdata/netboot-proxy-network.argv |  26 +-
 .../routed-network-dhcphost.argv   |  15 +
 .../routed-network-dhcphost.xml|  19 +
 tests/networkxml2argvdata/routed-network.argv  |  13 +-
 tests/networkxml2argvtest.c|  49 +-
 22 files changed, 822 insertions(+), 350 deletions(-)
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.argv
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.xml
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.argv
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.xml
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.argv
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.xml

-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/6] v6-6: add dnsmasq interface= parameter

[Gene Czarcinski]

This patch adds the interface= dnsmasq parameter to the dnsmasq
conf-file.  The relavant tests are updated.
---
 src/network/bridge_driver.c| 10 ++
 tests/networkxml2argvdata/isolated-network.argv|  1 +
 tests/networkxml2argvdata/nat-network-dns-hosts.argv   |  1 +
 .../nat-network-dns-srv-record-minimal.argv|  1 +
 tests/networkxml2argvdata/nat-network-dns-srv-record.argv  |  1 +
 tests/networkxml2argvdata/nat-network-dns-txt-record.argv  |  1 +
 tests/networkxml2argvdata/nat-network.argv |  1 +
 tests/networkxml2argvdata/netboot-network.argv |  1 +
 tests/networkxml2argvdata/netboot-proxy-network.argv   |  1 +
 tests/networkxml2argvdata/routed-network.argv  |  1 +
 10 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 508de3a..236d8f8 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -751,14 +751,8 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 }
 }
 
-/*
- * --interface does not actually work with dnsmasq  2.47,
- * due to DAD for ipv6 addresses on the interface.
- *
- * virCommandAddArgList(cmd, --interface, ipdef-bridge, NULL);
- *
- * So listen on all defined IPv[46] addresses
- */
+virBufferAsprintf(configbuf, interface=%s\n, network-def-bridge);
+
 for (ii = 0;
  (tmpipdef = virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, ii));
  ii++)
diff --git a/tests/networkxml2argvdata/isolated-network.argv 
b/tests/networkxml2argvdata/isolated-network.argv
index 042158b..abcde93 100644
--- a/tests/networkxml2argvdata/isolated-network.argv
+++ b/tests/networkxml2argvdata/isolated-network.argv
@@ -6,6 +6,7 @@ domain-needed
 local=//
 dhcp-option=3
 no-resolv
+interface=virbr2
 listen-address=192.168.152.1
 dhcp-range=192.168.152.2,192.168.152.254
 dhcp-leasefile=/var/lib/libvirt/dnsmasq/private.leases
diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv 
b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
index 91eb682..7dce6f9 100644
--- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv
@@ -6,5 +6,6 @@ domain-needed
 local=/example.com/
 domain=example.com
 expand-hosts
+interface=virbr0
 listen-address=192.168.122.1
 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv 
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
index d92497b..d87d438 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -5,6 +5,7 @@ except-interface=lo
 domain-needed
 local=//
 srv-host=name.tcp.
+interface=virbr0
 listen-address=192.168.122.1
 listen-address=192.168.123.1
 listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv 
b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
index d8846c2..53882fe 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv
@@ -5,6 +5,7 @@ except-interface=lo
 domain-needed
 local=//
 srv-host=name.tcp.test-domain-name,.,1024,10,10
+interface=virbr0
 listen-address=192.168.122.1
 listen-address=192.168.123.1
 listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv 
b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
index bf00513..cc3ed28 100644
--- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv
@@ -5,6 +5,7 @@ except-interface=lo
 domain-needed
 local=//
 txt-record=example,example value
+interface=virbr0
 listen-address=192.168.122.1
 listen-address=192.168.123.1
 listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/nat-network.argv 
b/tests/networkxml2argvdata/nat-network.argv
index d542bbc..431fffb 100644
--- a/tests/networkxml2argvdata/nat-network.argv
+++ b/tests/networkxml2argvdata/nat-network.argv
@@ -4,6 +4,7 @@ bind-interfaces
 except-interface=lo
 domain-needed
 local=//
+interface=virbr0
 listen-address=192.168.122.1
 listen-address=192.168.123.1
 listen-address=2001:db8:ac10:fe01::1
diff --git a/tests/networkxml2argvdata/netboot-network.argv 
b/tests/networkxml2argvdata/netboot-network.argv
index 4f5fedd..8405095 100644
--- a/tests/networkxml2argvdata/netboot-network.argv
+++ b/tests/networkxml2argvdata/netboot-network.argv
@@ -6,6 +6,7 @@ domain-needed
 local=/example.com/
 domain=example.com
 expand-hosts
+interface=virbr1
 listen-address=192.168.122.1
 dhcp-range=192.168.122.2,192.168.122.254
 dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases
diff --git 

[libvirt] [PATCH 1/6] v6-6: put dnsmasq parameters into a file

[Gene Czarcinski]

This patch changes how parameters are passed to dnsmasq.  Instead of
being on the command line, the parameters are put into a file (one
parameter per line) and a commandline --conf-file= specifies the
location of the file.  The file is located in the same directory as
the leases file.
---
 src/network/bridge_driver.c| 153 ++---
 src/network/bridge_driver.h|   7 +-
 tests/networkxml2argvdata/isolated-network.argv|  24 ++--
 .../networkxml2argvdata/nat-network-dns-hosts.argv |  14 +-
 .../nat-network-dns-srv-record-minimal.argv|  35 ++---
 .../nat-network-dns-srv-record.argv|  35 ++---
 .../nat-network-dns-txt-record.argv|  29 ++--
 tests/networkxml2argvdata/nat-network.argv |  27 ++--
 tests/networkxml2argvdata/netboot-network.argv |  28 ++--
 .../networkxml2argvdata/netboot-proxy-network.argv |  25 ++--
 tests/networkxml2argvdata/routed-network.argv  |  12 +-
 tests/networkxml2argvtest.c|  43 +-
 12 files changed, 245 insertions(+), 187 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c153d36..508de3a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -136,6 +136,16 @@ networkDnsmasqLeaseFileNameFunc 
networkDnsmasqLeaseFileName =
 networkDnsmasqLeaseFileNameDefault;
 
 static char *
+networkDnsmasqConfigFileName(const char *netname)
+{
+char *conffile;
+
+ignore_value(virAsprintf(conffile, DNSMASQ_STATE_DIR /%s.conf,
+ netname));
+return conffile;
+}
+
+static char *
 networkRadvdPidfileBasename(const char *netname)
 {
 /* this is simple but we want to be sure it's consistently done */
@@ -163,6 +173,7 @@ networkRemoveInactive(struct network_driver *driver,
 char *leasefile = NULL;
 char *radvdconfigfile = NULL;
 char *radvdpidbase = NULL;
+char *configfile = NULL;
 dnsmasqContext *dctx = NULL;
 virNetworkDefPtr def = virNetworkObjGetPersistentDef(net);
 
@@ -178,12 +189,16 @@ networkRemoveInactive(struct network_driver *driver,
 if (!(radvdconfigfile = networkRadvdConfigFileName(def-name)))
 goto no_memory;
 
-if (!(radvdpidbase = networkRadvdPidfileBasename(def-name)))
+if (!(radvdconfigfile = networkRadvdConfigFileName(def-name)))
+goto no_memory;
+
+if (!(configfile = networkDnsmasqConfigFileName(def-name)))
 goto no_memory;
 
 /* dnsmasq */
 dnsmasqDelete(dctx);
 unlink(leasefile);
+unlink(configfile);
 
 /* radvd */
 unlink(radvdconfigfile);
@@ -196,6 +211,7 @@ networkRemoveInactive(struct network_driver *driver,
 
 cleanup:
 VIR_FREE(leasefile);
+VIR_FREE(configfile);
 VIR_FREE(radvdconfigfile);
 VIR_FREE(radvdpidbase);
 dnsmasqContextFree(dctx);
@@ -610,14 +626,15 @@ networkBuildDnsmasqHostsfile(dnsmasqContext *dctx,
 return 0;
 }
 
-
+/* build the dnsmasq conf file contents */
 static int
-networkBuildDnsmasqArgv(virNetworkObjPtr network,
+networkDnsmasqConfContents(virNetworkObjPtr network,
 virNetworkIpDefPtr ipdef,
 const char *pidfile,
-virCommandPtr cmd,
+char **configstr,
 dnsmasqContext *dctx)
 {
+virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
 int r, ret = -1;
 int nbleases = 0;
 int ii;
@@ -627,6 +644,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
 char *recordPriority = NULL;
 virNetworkIpDefPtr tmpipdef;
 
+*configstr = NULL;
+
 /*
  * NB, be careful about syntax for dnsmasq options in long format.
  *
@@ -646,28 +665,22 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
  * very explicit on this.
  */
 
-/*
- * Needed to ensure dnsmasq uses same algorithm for processing
- * multiple namedriver entries in /etc/resolv.conf as GLibC.
- */
-virCommandAddArgList(cmd, --strict-order, --bind-interfaces, NULL);
-
+/* create dnsmasq config file appropriate for this network */
+virBufferAsprintf(configbuf, # dnsmasq conf file created by libvirt\n
+strict-order\n
+bind-interfaces\n
+except-interface=lo\n
+domain-needed\n
+local=/%s/\n,
+network-def-domain ? network-def-domain : );
 if (network-def-domain)
-virCommandAddArgPair(cmd, --domain, network-def-domain);
-/* need to specify local even if no domain specified */
-virCommandAddArgFormat(cmd, --local=/%s/,
-   network-def-domain ? network-def-domain : );
-virCommandAddArg(cmd, --domain-needed);
+virBufferAsprintf(configbuf, 
+domain=%s\n
+expand-hosts\n,
+network-def-domain);
 
 if (pidfile)
-

[libvirt] [PATCH 4/6] v1: get, parse, and save dnsmasq version id

[Gene Czarcinski]

Use dnsmasq --version to obtain dnsmasq's version id and
save the major/minor values.  This is a separate patch since other
patches will depend on it.
---
 src/conf/network_conf.h |  2 ++
 src/network/bridge_driver.c | 35 +--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3e46304..cfc49af 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -218,6 +218,8 @@ struct _virNetworkObj {
 unsigned int active : 1;
 unsigned int autostart : 1;
 unsigned int persistent : 1;
+unsigned int dnsmasqMajor;
+unsigned int dnsmasqMinor;
 
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0c4c794..19610f2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -968,21 +968,51 @@ cleanup:
 return ret;
 }
 
+#define DNSMASQ_VERSION_PREFIX Dnsmasq version 
+
 static int
 networkStartDhcpDaemon(virNetworkObjPtr network)
 {
 virCommandPtr cmd = NULL;
+const char *cmdname = DNSMASQ;
+char *tmp, *version = NULL;
+unsigned int  major = 0, minor = 0;
 char *pidfile = NULL;
 char *testconfigstr = NULL;
 int ret = -1;
 dnsmasqContext *dctx = NULL;
 
 if (!virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, 0)) {
-/* no IPv6 addresses, so we don't need to run radvd */
+/* no IP addresses, so we don't need to run */
 ret = 0;
 goto cleanup;
 }
-VIR_INFO(starting dhcp daemon (dnsmasq));
+
+if (!virFileIsExecutable(cmdname)) {
+VIR_WARN(file %s missing or not executable, cmdname);
+goto cleanup;
+}
+VIR_INFO(starting dhcp daemon (%s), cmdname);
+
+cmd = virCommandNew(cmdname);
+virCommandAddArg(cmd, --version);
+virCommandSetOutputBuffer(cmd, version);
+if (virCommandRun(cmd, NULL)  0)
+goto cleanup;
+virCommandFree(cmd);
+cmd = NULL;
+
+if ((version!=NULL) 
+(strncmp(version, DNSMASQ_VERSION_PREFIX, 
strlen(DNSMASQ_VERSION_PREFIX))==0)) {
+tmp = version + strlen(DNSMASQ_VERSION_PREFIX);
+if (virStrToLong_ui(tmp, tmp, 10, major) = 0) {
+if ((*tmp == '.') 
+virStrToLong_ui(tmp + 1, tmp, 10, minor) = 0) {
+network-dnsmasqMajor = major;
+network-dnsmasqMinor = minor;
+}
+}
+}
 
 if (virFileMakePath(NETWORK_PID_DIR)  0) {
 virReportSystemError(errno,
@@ -1043,6 +1073,7 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
 ret = 0;
 cleanup:
 VIR_FREE(pidfile);
+VIR_FREE(version);
 virCommandFree(cmd);
 dnsmasqContextFree(dctx);
 return ret;
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 5/6] v1: use dnsmasq instead of radvd to handle RA service

[Gene Czarcinski]

This patch tests the version of dnsmasq.  If (major  2)
or ((major==2) and (minor = 63)), then use dnsmasq.  Otherwise,
use radvd to provide the RA service.
---
 src/conf/network_conf.h|  1 +
 src/network/bridge_driver.c| 36 ++
 tests/networkxml2argvdata/dhcp6-network.argv   |  1 +
 tests/networkxml2argvdata/nat-network-dhcp6.argv   |  1 +
 .../nat-network-dns-srv-record-minimal.argv|  1 +
 .../nat-network-dns-srv-record.argv|  1 +
 .../nat-network-dns-txt-record.argv|  1 +
 tests/networkxml2argvdata/nat-network.argv |  1 +
 .../routed-network-dhcphost.argv   |  1 +
 tests/networkxml2argvtest.c|  1 +
 10 files changed, 45 insertions(+)

diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index cfc49af..39fffd7 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -220,6 +220,7 @@ struct _virNetworkObj {
 unsigned int persistent : 1;
 unsigned int dnsmasqMajor;
 unsigned int dnsmasqMinor;
+unsigned int dnsmasqRA;
 
 virNetworkDefPtr def; /* The current definition */
 virNetworkDefPtr newDef; /* New definition to activate at shutdown */
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 19610f2..2230268 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -893,6 +893,26 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 virBufferAsprintf(configbuf, addn-hosts=%s\n,
  dctx-addnhostsfile-path);
 
+/* we are doing RA instead of radvd */
+if (network-dnsmasqRA) {
+if (dhcp6flag)
+virBufferAsprintf(configbuf, enable-ra\n);
+else {
+char *bridgeaddr = NULL;
+ipdef = virNetworkDefGetIpByIndex(network-def, AF_INET6, 0);
+if (ipdef) {
+bridgeaddr = virSocketAddrFormat(ipdef-address);
+if (bridgeaddr) {
+virBufferAsprintf(configbuf,
+dhcp-range=%s,ra-only\n, bridgeaddr);
+}
+}
+if ((!ipdef) || (!bridgeaddr))
+ VIR_WARN(IPv6 invalid configuration for %s, 
network-def-name);
+VIR_FREE(bridgeaddr);
+}
+}
+
 if (!(*configstr = virBufferContentAndReset(configbuf))) {
 virReportOOMError();
 goto cleanup;
@@ -1014,6 +1034,12 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
 }
 }
 
+/* is radvd or dnsmasq to handle RA? */
+if ((network-dnsmasqMajor  2) ||
+((network-dnsmasqMajor == 2) 
+ (network-dnsmasqMinor = 63)))
+network-dnsmasqRA = 1;
+
 if (virFileMakePath(NETWORK_PID_DIR)  0) {
 virReportSystemError(errno,
  _(cannot create directory %s),
@@ -1293,6 +1319,12 @@ networkStartRadvd(virNetworkObjPtr network)
 virCommandPtr cmd = NULL;
 int ret = -1;
 
+/* is dnsmasq handling RA */
+if (network-dnsmasqRA) {
+ret = 0;
+goto cleanup;
+}
+
 network-radvdPid = -1;
 
 if (!virNetworkDefGetIpByIndex(network-def, AF_INET6, 0)) {
@@ -1370,6 +1402,10 @@ cleanup:
 static int
 networkRefreshRadvd(virNetworkObjPtr network)
 {
+/* is dnsmasq handling RA */
+if (network-dnsmasqRA)
+return 0;
+
 /* if there's no running radvd, just start it */
 if (network-radvdPid = 0 || (kill(network-radvdPid, 0)  0))
 return networkStartRadvd(network);
diff --git a/tests/networkxml2argvdata/dhcp6-network.argv 
b/tests/networkxml2argvdata/dhcp6-network.argv
index 6697833..66ec430 100644
--- a/tests/networkxml2argvdata/dhcp6-network.argv
+++ b/tests/networkxml2argvdata/dhcp6-network.argv
@@ -14,3 +14,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=240
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
+enable-ra
diff --git a/tests/networkxml2argvdata/nat-network-dhcp6.argv 
b/tests/networkxml2argvdata/nat-network-dhcp6.argv
index 14256ae..c56fff1 100644
--- a/tests/networkxml2argvdata/nat-network-dhcp6.argv
+++ b/tests/networkxml2argvdata/nat-network-dhcp6.argv
@@ -17,3 +17,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=493
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts
+enable-ra
diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv 
b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
index 45a658b..26f283e 100644
--- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
+++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv
@@ -17,3 +17,4 @@ dhcp-leasefile=/var/lib/libvirt/dnsmasq/default.leases
 dhcp-lease-max=253
 dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile
 

[libvirt] [PATCH 6/6] v1: for DHCPv6, add dnsmasq version check

[Gene Czarcinski]

If dnsmasq version is = 2.64, then IPV6 dhcp-range and
dhcp-host are proccessed/supported.  Otherwise, error out
with message that DHCPv6 range and host not supported by
the installed dnsmasq version.
---
 src/network/bridge_driver.c | 15 ++-
 tests/networkxml2argvtest.c |  2 ++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2230268..a0c85b5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -654,7 +654,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 char *recordWeight = NULL;
 char *recordPriority = NULL;
 virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def;
-bool dhcp4flag, dhcp6flag;
+bool dhcp4flag, dhcp6flag, dhcp6_OK;
 
 *configstr = NULL;
 
@@ -776,6 +776,13 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 VIR_FREE(ipaddr);
 }
 
+/* check to is if dnsmasq version = 2.64 so that DHCPv6 is OK */
+dhcp6_OK = false;
+if (network-dnsmasqMajor  2)
+dhcp6_OK = true;
+if ((network-dnsmasqMajor == 2)  (network-dnsmasqMinor  63))
+dhcp6_OK = true;
+
 /* Find the first dhcp for both IPv4 and IPv6 */
 for (ii = 0, ipv4def = NULL, ipv6def = NULL, dhcp4flag = false, dhcp6flag 
= false;
  (ipdef = virNetworkDefGetIpByIndex(network-def, AF_UNSPEC, ii));
@@ -790,6 +797,12 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 }
 if (VIR_SOCKET_ADDR_IS_FAMILY(ipdef-address, AF_INET6)) {
 if (ipdef-nranges || ipdef-nhosts) {
+if (!dhcp6_OK) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(Invlaid to specify DHCPv6 range or host for dnsmasq 
version %u.%u),
+   network-dnsmasqMajor, network-dnsmasqMinor);
+goto cleanup;
+}
 if (!ipv6def) {
 ipv6def = ipdef;
 dhcp6flag = true;
diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c
index 413ba80..a64da65 100644
--- a/tests/networkxml2argvtest.c
+++ b/tests/networkxml2argvtest.c
@@ -38,6 +38,8 @@ static int testCompareXMLToArgvFiles(const char *inxml, const 
char *outargv) {
 goto fail;
 
 obj-def = dev;
+obj-dnsmasqMajor = 2;
+obj-dnsmasqMinor = 64;
 obj-dnsmasqRA = 1;
 dctx = dnsmasqContextNew(dev-name, /var/lib/libvirt/dnsmasq);
 
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/6] v6-7: Add support for DHCPv6

[Gene Czarcinski]

This support includes IPv6 dhcp-range= and dhcp-host=
for one IPv6 subnetwork on one interface.

The parameter tests have been updated to add some new
DHCPv6 tests.  Three new tests (6 files) were added.

The xml format html documentation has been updated to
reflect support for IPv6 DHCP.

This patch include a bugfix for ipv4flag and ipv6flag
initialization.
---
 docs/formatnetwork.html.in | 108 ++-
 src/conf/network_conf.c| 100 ---
 src/network/bridge_driver.c| 326 +
 src/util/dnsmasq.c |   9 +-
 tests/networkxml2argvdata/dhcp6-network.argv   |  16 +
 tests/networkxml2argvdata/dhcp6-network.xml|  16 +
 tests/networkxml2argvdata/isolated-network.argv|   2 +-
 tests/networkxml2argvdata/nat-network-dhcp6.argv   |  19 ++
 tests/networkxml2argvdata/nat-network-dhcp6.xml|  26 ++
 .../nat-network-dns-srv-record-minimal.argv|   2 +-
 .../nat-network-dns-srv-record.argv|   2 +-
 .../nat-network-dns-txt-record.argv|   2 +-
 tests/networkxml2argvdata/nat-network.argv |   2 +-
 tests/networkxml2argvdata/netboot-network.argv |   8 +-
 .../networkxml2argvdata/netboot-proxy-network.argv |   4 +-
 .../routed-network-dhcphost.argv   |  14 +
 .../routed-network-dhcphost.xml|  19 ++
 tests/networkxml2argvtest.c|   3 +
 18 files changed, 498 insertions(+), 180 deletions(-)
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.argv
 create mode 100644 tests/networkxml2argvdata/dhcp6-network.xml
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.argv
 create mode 100644 tests/networkxml2argvdata/nat-network-dhcp6.xml
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.argv
 create mode 100644 tests/networkxml2argvdata/routed-network-dhcphost.xml

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..b91672a 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -577,8 +577,10 @@
 dotted-decimal format, or an IPv6 address in standard
 colon-separated hexadecimal format, that will be configured on
 the bridge
-device associated with the virtual network. To the guests this
-address will be their default route. For IPv4 addresses, the 
codenetmask/code
+device associated with the virtual network. To the guests this IPv4
+address will be their IPv4 default route.  For IPv6, the default route 
is
+established via Router Advertisement.
+For IPv4 addresses, the codenetmask/code
 attribute defines the significant bits of the network address,
 again specified in dotted-decimal format.  For IPv6 addresses,
 and as an alternate method for IPv4 addresses, you can specify
@@ -587,10 +589,13 @@
 could also be given as codeprefix='24'/code. The 
codefamily/code
 attribute is used to specify the type of address - 'ipv4' or 'ipv6'; 
if no
 codefamily/code is given, 'ipv4' is assumed. A network can have 
more than
-one of each family of address defined, but only a single address can 
have a
-codedhcp/code or codetftp/code element. span 
class=sinceSince 0.3.0;
+one of each family of address defined, but only a single IPv4 address 
can have a
+codedhcp/code or codetftp/code element. span 
class=sinceSince 0.3.0 /span
 IPv6, multiple addresses on a single network, codefamily/code, and
-codeprefix/code since 0.8.7/span
+codeprefix/code. span class=sinceSince 0.8.7/span  In 
addition
+to the one IPv4 address which has a codedhcp/code definition, one 
IPv6
+address can have a codedhcp/code definition.
+span class=since Since 1.0.1/span
 dl
   dtcodetftp/code/dt
   ddImmediately within
@@ -611,27 +616,46 @@
 codedhcp/code element is not supported for IPv6, and
 is only supported on a single IP address per network for IPv4.
 span class=sinceSince 0.3.0/span
+The codedhcp/code element is now supported for IPv6.
+Again, there is a restriction that only one IPv6 address definition
+is able to have a codedhcp/code element.
+span class=sinceSince 1.0.1/span
 dl
   dtcoderange/code/dt
   ddThe codestart/code and codeend/code attributes on the
 coderange/code element specify the boundaries of a pool of
-IPv4 addresses to be provided to DHCP clients. These two 
addresses
+addresses to be provided to DHCP clients. These two addresses
 must lie within the scope of the network defined on the parent
-codeip/code element.  span class=sinceSince 
0.3.0/span
+codeip/code element.  

Re: [libvirt] Proposed: always allow packets internal to an interface

[Gene Czarcinski]

On 11/08/2012 02:41 PM, Laine Stump wrote:

On 11/07/2012 04:25 PM, Gene Czarcinski wrote:

On 11/06/2012 11:23 AM, Gene Czarcinski wrote:

ip6tables -I FORWARD 1 -i  virbr18 -o  virbr18  -j  ACCEPT

This one currently isn't getting added, because
networkAddGeneralIp6tablesRules() returns immediately if there are no
ipv6 addresses defined for the network.

Note that up until now, unless someone had an ipv6 address defined
for a
network, ip6tables was never called, so theoretically you could run
libvirtd without having ip6tables installed on your machine, but with
this change *all* networks would fail to start if the ip6tables binary
was missing. That *shouldn't* be a problem because (at least on
Fedora/RHEL/CentOS) it is a part of the same package as iptables, but
I've seen strange setups in the last few years - in particular I recall
one Gentoo user whose networks were mysteriously failing, and it was
because he had built the iproute package with some sort of minimal
configuration that's available on Gentoo, causing something or other to
fail in a mysterious way (compounded in troubleshooting by the fact
that
none of us would ever have expected such a thing).

How about a configure/compile time option for those systems which may
not have ip6tables ... the default being that ip6tables is assumed.

ping

Sorry, I've been overwhelmed and am churning.


OK, I have not heard a plain yes or no on this.

IPv4 and IPv6 networks are suppose to have the same (more or less)
functionality so why isn't this OK.

Maintaining backward compatibility, both API and operational. In the
past it wasn't the case that we simply did nothing about ipv6 on
libvirt's networks, instead we explicitly set a sysctl to *disable* it.
That must have been done for some reason. That reason may no longer be
valid, but we don't know that yet (it happened before I was around). If
the reason is no longer valid, we can go ahead as you suggest (and I
would say we don't even need an option to not have ip6tables, just force
people to build the full iptables package as God intended :-P). If the
reason *is* still valid, then we need to only enable the ipv6 sysctl and
add the ip6tables rule in response to some new flag attribute in the
network config.


If you do want to give someone the option of running without
ip6tables, fine make it a compile-time option.

Actually I want to hear some historical info about why ipv6 was
explicitly *disabled* with sysctl on libvirt's networks in the past.
Maybe it's time to change that default, but I don't want to do it
lightly - it may even have been done in response to a CVE. (danpb or DV
would probably have the best insight about this. I'll point them at this
thread.)

New functionality is great, but you can't upset working systems. (I know
I'm very tedious and am sounding like a broken record, but this is a
topic that libvirt takes *very* seriously; the API must always be 100%
backward compatible, and consciously programmed behavior should always
remain the same.)

I  believe it is time for someone who has the project memory to chime in 
on this.


It also might be a good idea, if individuals who are knowledgeable in 
distributions other than Fedora and RHEL, to add their perspective


On the other hand, while there may have been valid reasons to do this 
originally, it may be continuing because this is the way we always did 
it.


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 02:41:29PM -0500, Laine Stump wrote:
 On 11/07/2012 04:25 PM, Gene Czarcinski wrote:
  IPv4 and IPv6 networks are suppose to have the same (more or less)
  functionality so why isn't this OK.
 
 Maintaining backward compatibility, both API and operational. In the
 past it wasn't the case that we simply did nothing about ipv6 on
 libvirt's networks, instead we explicitly set a sysctl to *disable* it.
 That must have been done for some reason. That reason may no longer be
 valid, but we don't know that yet (it happened before I was around). If
 the reason is no longer valid, we can go ahead as you suggest (and I
 would say we don't even need an option to not have ip6tables, just force
 people to build the full iptables package as God intended :-P). If the
 reason *is* still valid, then we need to only enable the ipv6 sysctl and
 add the ip6tables rule in response to some new flag attribute in the
 network config.

If you don't disable IPv6 on the bridge device, then when starting the
network device, the kernel will auto-assign an IPv6 link local address,
which the guest can then use to communicate with the host. In the IPv4
case, if you don't specify any ip address, there is no link local
like address present, so there's no connectivity between guest and
host. So explicitly disabling IPv6 is in fact required in order to
give consistent behaviour between IPv6 and IPv4.

I've no objections to anyone adding a new 'ipv6=on|off' attribute to
the network XML so that admins can explicitly choosen whether to allow
IPv6, indepedently of whether any ip element is set with an IPv6 address.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] storage: allow metadata preallocation when creating qcow2 images

[Eric Blake]

On 11/08/2012 09:28 AM, Daniel P. Berrange wrote:
 
 I've thought about doing it this way before, but I always felt it was
 a bit of a gross hack. It would be nice to keep the 'allocation' element
 as purely describing the data sector allocation. I was recently wondering
 if we should just add a flag to the virStorageVolCreate like
 
   VIR_STORAGE_VOL_PREALLOC_METADATA

I agree that adding a new flag is cleaner; thankfully,
virStorageVolCreateXML has a flags argument ready to use.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote:
   
 https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
 the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
 'start', 'stop', 'restart'; might be easier to remember than the
 current mix of 'start', 'shutdown', 'reboot'.

 * tools/virsh-domain.c (domManagementCmds): Add other command names.
 * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
 ---

 This patch documents both spellings.  An alternative would be to
 leave the alternate spellings as hidden aliases (virsh has support
 for that), but still mention them in virsh.pod (see how we did an
 alias for nodedev-dettach, for reference).
 

 NACK to this patch. I think the current command names are good.
 Creating duplicates will make life worse. First, it creates
 divergance from the similarly named commands for networks,
 storage and other objects.

Hmm, that is a very good observation, one I hadn't considered.  Agreed
that consistency with other commands is certainly important, where
'booting' a network e.g. is not very intuitive.

Regards,
Jim

  It also means scripts written again
 the new commands will not work with existing libvirt.

 I actually think that shutdown  reboot are *better* names
 than restart and stop.

 If we wanted to replace any existing names, then the 'create'
 and 'destroy' names are the ones to replace, and for those I
 would expect to use 'boot' and 'stop'. I still don't thin
 we should do that either, due to creating inconsistency with
 other commands.

 Daniel
   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Gene Czarcinski]

On 11/08/2012 04:44 PM, Daniel P. Berrange wrote:

On Thu, Nov 08, 2012 at 02:41:29PM -0500, Laine Stump wrote:

On 11/07/2012 04:25 PM, Gene Czarcinski wrote:

IPv4 and IPv6 networks are suppose to have the same (more or less)
functionality so why isn't this OK.

Maintaining backward compatibility, both API and operational. In the
past it wasn't the case that we simply did nothing about ipv6 on
libvirt's networks, instead we explicitly set a sysctl to *disable* it.
That must have been done for some reason. That reason may no longer be
valid, but we don't know that yet (it happened before I was around). If
the reason is no longer valid, we can go ahead as you suggest (and I
would say we don't even need an option to not have ip6tables, just force
people to build the full iptables package as God intended :-P). If the
reason *is* still valid, then we need to only enable the ipv6 sysctl and
add the ip6tables rule in response to some new flag attribute in the
network config.

If you don't disable IPv6 on the bridge device, then when starting the
network device, the kernel will auto-assign an IPv6 link local address,
which the guest can then use to communicate with the host. In the IPv4
case, if you don't specify any ip address, there is no link local
like address present, so there's no connectivity between guest and
host. So explicitly disabling IPv6 is in fact required in order to
give consistent behaviour between IPv6 and IPv4.

I've no objections to anyone adding a new 'ipv6=on|off' attribute to
the network XML so that admins can explicitly choosen whether to allow
IPv6, indepedently of whether any ip element is set with an IPv6 address.

I hear what you are saying but I am not sure I understand it because 
some simple testing I did resulted in exactly what I wanted.


1. Configure and start ad virtual network interface such as:
network
  namenogw/name
  uuid7a3b7497-1ec7-8aef-6d5c-38dff9109e93/uuid
  bridge name='virbr19' stp='on' delay='0' /
  mac address='52:54:00:08:10:43'/
/network

ip addr shows the following:
44: virbr19: NO-CARRIER,BROADCAST,MULTICAST,UP mtu 1500 qdisc noqueue 
state DOWN

link/ether 52:54:00:08:10:43 brd ff:ff:ff:ff:ff:ff
45: virbr19-nic: BROADCAST,MULTICAST mtu 1500 qdisc pfifo_fast master 
virbr19 state DOWN qlen 500

link/ether 52:54:00:08:10:43 brd ff:ff:ff:ff:ff:ff

and I added a rule to ip6tables resulting in:
-A FORWARD -i virbr19 -o virbr19 -j ACCEPT

2. Take two F17 virtual guest systems and configure them with nogw on 
a network interface.


3. Start them up and manually configure the NIC with the nogw network 
for fd00:1:1:1::2/64 and fd00:1:1:1::3/64


4. try doing a ping6 between the two ... works fine.

Now, all I am asking for is to have the above ip6table rule added 
automatically (along with the standard rejects).


The reult is a very private IPv6 network between the virtual guest systems.

BTW, for sysctl -a | grep virbr19 | grep disable_ipv6, the result is:
net.ipv6.conf.virbr19.disable_ipv6 = 1
net.ipv6.conf.virbr19-nic.disable_ipv6 = 0

Just for info, this is all F17 with libvirt-1.0.0+ my bunch of patches.

Now, what am I missing?  What do I not understand?

Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemu: refactor graphics code to not hardcode a single display

[Eric Blake]

On 11/08/2012 07:04 AM, Daniel P. Berrange wrote:
 On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
 The check for a single display remains so no new functionality is added.
 ---

 Concerning both patches, I've tested running with all three
 simultaneously (sdl+spice+vnc) and with spice+vnc, using a
 single qxl device.

  src/qemu/qemu_command.c | 2425 
 ---
  src/qemu/qemu_process.c |   70 +-
  2 files changed, 1259 insertions(+), 1236 deletions(-)
 
 I'm not really sure what has happened here, but you have got
 an absolutely enourmous diff here. I would expect this patch
 to only affect the small part of qemuBuildCommandLine that
 actually deals with graphics devices. As it is now, it is
 impossible to review this patch I'm afraid :-(

'git diff --patience' to the rescue:

$ git diff HEAD^ --stat
 src/qemu/qemu_command.c | 2425
---
 src/qemu/qemu_process.c |   70 +-
 2 files changed, 1259 insertions(+), 1236 deletions(-)
$ git diff HEAD^ --stat --patience
 src/qemu/qemu_command.c | 647
+---
 src/qemu/qemu_process.c |  70 +++---
 2 files changed, 370 insertions(+), 347 deletions(-)

I'll send the patience version of the patch as a followup for easier review.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2 RESEND] qemu: refactor graphics code to not hardcode a single display

[Eric Blake]

From: Alon Levy al...@redhat.com

The check for a single display remains so no new functionality is added.
---

No change to the patch itself, just the use of the --patience flag
to make review much easier.

 src/qemu/qemu_command.c | 647 +---
 src/qemu/qemu_process.c |  70 +++---
 2 files changed, 370 insertions(+), 347 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1e96982..f9e4d4d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4410,6 +4410,334 @@ error:
 return -1;
 }

+enum {
+OK=0,
+ERROR=1,
+NO_MEMORY=2,
+};
+
+static int
+qemuBuildGraphicsCommandLine(struct qemud_driver *driver,
+ virCommandPtr cmd,
+ virDomainDefPtr def,
+ qemuCapsPtr caps,
+ virDomainGraphicsDefPtr graphics)
+{
+if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
+virBuffer opt = VIR_BUFFER_INITIALIZER;
+
+if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(vnc graphics are not supported with this QEMU));
+return ERROR;
+}
+
+if (graphics-data.vnc.socket ||
+driver-vncAutoUnixSocket) {
+
+if (!graphics-data.vnc.socket 
+virAsprintf(graphics-data.vnc.socket,
+%s/%s.vnc, driver-libDir, def-name) == -1) {
+return NO_MEMORY;
+}
+
+virBufferAsprintf(opt, unix:%s,
+  graphics-data.vnc.socket);
+
+} else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
+const char *listenNetwork;
+const char *listenAddr = NULL;
+char *netAddr = NULL;
+bool escapeAddr;
+int ret;
+
+switch (virDomainGraphicsListenGetType(graphics, 0)) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
+listenAddr = virDomainGraphicsListenGetAddress(graphics, 0);
+break;
+
+case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0);
+if (!listenNetwork)
+break;
+ret = networkGetNetworkAddress(listenNetwork, netAddr);
+if (ret = -2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(network-based listen not possible, 

+   network driver not present));
+return 1;
+}
+if (ret  0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(listen network '%s' had no usable 
address),
+   listenNetwork);
+return 1;
+}
+listenAddr = netAddr;
+/* store the address we found in the graphics element so it 
will
+ * show up in status. */
+if (virDomainGraphicsListenSetAddress(graphics, 0,
+  listenAddr, -1, false)  
0)
+   return 1;
+break;
+}
+
+if (!listenAddr)
+listenAddr = driver-vncListen;
+
+escapeAddr = strchr(listenAddr, ':') != NULL;
+if (escapeAddr)
+virBufferAsprintf(opt, [%s], listenAddr);
+else
+virBufferAdd(opt, listenAddr, -1);
+virBufferAsprintf(opt, :%d,
+  graphics-data.vnc.port - 5900);
+
+VIR_FREE(netAddr);
+} else {
+virBufferAsprintf(opt, %d,
+  graphics-data.vnc.port - 5900);
+}
+
+if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) {
+if (graphics-data.vnc.auth.passwd ||
+driver-vncPassword)
+virBufferAddLit(opt, ,password);
+
+if (driver-vncTLS) {
+virBufferAddLit(opt, ,tls);
+if (driver-vncTLSx509verify) {
+virBufferAsprintf(opt, ,x509verify=%s,
+  driver-vncTLSx509certdir);
+} else {
+virBufferAsprintf(opt, ,x509=%s,
+  driver-vncTLSx509certdir);
+}
+}
+
+if (driver-vncSASL) {
+virBufferAddLit(opt, ,sasl);
+
+if (driver-vncSASLdir)
+virCommandAddEnvPair(cmd, SASL_CONF_DIR,
+ driver-vncSASLdir);
+
+/* TODO: Support ACLs later */
+}
+}
+
+virCommandAddArg(cmd, -vnc);
+virCommandAddArgBuffer(cmd, opt);
+if 

Re: [libvirt] [PATCH 1/3] nodeinfo: Add check and workaround to guarantee valid cpu topologies

[Eric Blake]

On 11/07/2012 08:22 AM, Peter Krempa wrote:
 Lately there were a few reports of the output of the virsh nodeinfo
 command being inaccurate. This patch ties to avoid that by checking if

s/ties/tries/

 the topology actualy makes sense. If it doesn't we then report a

s/actualy/actually/

 synthetic topology that indicates to the user that the host capabilities
 should be checked for the actual topology.
 ---
  src/nodeinfo.c | 37 +++--
  1 file changed, 31 insertions(+), 6 deletions(-)

You've convinced me that we need this; and we have already documented
that nodes=1 is a hint that the user must verify with the capability XML
for accurate results anyway.  ACK if you can answer one question:

 @@ -531,6 +539,23 @@ done:
  goto cleanup;
  }
 
 +/* Now check if the topology makes sense. There are machines that don't
 + * expose their real number of nodes or for example the AMD Bulldozer
 + * architecture that exposes their Clustered integer core modules as both
 + * threads and cores. This approach throws off our detection. 
 Unfortunately
 + * the nodeinfo structure isn't designed to carry the full topology so
 + * we're going to lie about the detected topology to notify the user
 + * to check the host capabilities for the actual topology. */
 +if ((nodeinfo-nodes *
 + nodeinfo-sockets *
 + nodeinfo-cores *
 + nodeinfo-threads) != (nodeinfo-cpus + offline)) {
 +nodeinfo-nodes = 1;
 +nodeinfo-sockets = nodeinfo-cpus;
 +nodeinfo-cores = 1;

Would it be any better to swap these and expose sockets == 1 and cores
== cpus when we fake things?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/3] nodeinfotest: Add test data for 2 processor host with broken NUMA

[Eric Blake]

On 11/07/2012 08:22 AM, Peter Krempa wrote:
 This test data was gathered on an AMD MagnyCours machine that reports it
 has only one NUMA node although the hardware is consisting of 4. As
 duplicate core id's are ignored the reported topology was bogous. This
 should be fixed by the previous patch.
 
 Reported and data provided by George-Cristian Bîrzan.
 ---

  tests/nodeinfotest.c   |   1 +

Everything else is test data for this one new test.  ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] nodeinfotest: Add test data from a AMD bulldozer machine.

[Eric Blake]

On 11/07/2012 08:22 AM, Peter Krempa wrote:
 The AMD Bulldozer architecture uses so called Clustered integer core
 modules that count both as threads and cores. This patch expects the
 cpu to be detected using the new fallback condition otherwise twice the
 number of processors would be detected.
 ---

  tests/nodeinfotest.c   |1 +

Again, everything else is supporting data for this one new test.  ACK.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] nodeinfo: Add check and workaround to guarantee valid cpu topologies

[Peter Krempa]

On 11/08/12 23:55, Eric Blake wrote:

On 11/07/2012 08:22 AM, Peter Krempa wrote:

Lately there were a few reports of the output of the virsh nodeinfo
command being inaccurate. This patch ties to avoid that by checking if


s/ties/tries/


the topology actualy makes sense. If it doesn't we then report a


s/actualy/actually/


synthetic topology that indicates to the user that the host capabilities
should be checked for the actual topology.
---
  src/nodeinfo.c | 37 +++--
  1 file changed, 31 insertions(+), 6 deletions(-)


You've convinced me that we need this; and we have already documented
that nodes=1 is a hint that the user must verify with the capability XML
for accurate results anyway.  ACK if you can answer one question:


@@ -531,6 +539,23 @@ done:
  goto cleanup;
  }

+/* Now check if the topology makes sense. There are machines that don't
+ * expose their real number of nodes or for example the AMD Bulldozer
+ * architecture that exposes their Clustered integer core modules as both
+ * threads and cores. This approach throws off our detection. Unfortunately
+ * the nodeinfo structure isn't designed to carry the full topology so
+ * we're going to lie about the detected topology to notify the user
+ * to check the host capabilities for the actual topology. */
+if ((nodeinfo-nodes *
+ nodeinfo-sockets *
+ nodeinfo-cores *
+ nodeinfo-threads) != (nodeinfo-cpus + offline)) {
+nodeinfo-nodes = 1;
+nodeinfo-sockets = nodeinfo-cpus;
+nodeinfo-cores = 1;


Would it be any better to swap these and expose sockets == 1 and cores
== cpus when we fake things?



Hm the first version actually had it that way. I think we should tweak 
the docs in that case.


http://libvirt.org/html/libvirt-libvirt.html#virNodeInfo

When we're lying it's not that much important in which field we're doing 
so. Normally it could have some performance implications but in this 
case everything is off anyways.


Viktor in his review is having the same opinion plus a good catch about 
reporting the topology also with offline processors, so that thing 
should be changed.


Do you have any suggestions about tweaking the docs? I can't think of 
anything good right now.


Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] storage: fix broken backing chain

[Eric Blake]

On 11/07/2012 06:53 AM, Philipp Hahn wrote:
 82507838 refactured the code to keep both the raw and canonicalized form

s/refactured/refactored/

 of the backingStore, which breaks badly when the storage pool contains a
 storage volume, which is missing its backing store file:
  # ./daemon/libvirtd -l
  2012-11-07 12:43:33.279+: 22175: info : libvirt version: 1.0.0
  2012-11-07 12:43:33.279+: 22175: error : absolutePathFromBaseFile:542 : 
 Can't canonicalize path '/var/lib/libvirt/images/base.qcow2': No such file or 
 directory
  2012-11-07 12:43:33.280+: 22175: error : storageDriverAutostart:115 : 
 Failed to autostart storage pool 'default': Can't canonicalize path 
 '/var/lib/libvirt/images/base.qcow2': No such file or directory
 
 This is because virStorageFileGetMetadataFromBuf() aborts with -1 if the
 filename of the backingStore can not be canonicalized:
  #0  absolutePathFromBaseFile () at util/storage_file.c:541
  #1  virStorageFileGetMetadataFromBuf () at util/storage_file.c:728
  #2  virStorageFileGetMetadataFromFD () at util/storage_file.c:932
  #3  virStorageBackendProbeTarget () at storage/storage_backend_fs.c:94
  #4  virStorageBackendFileSystemRefresh () at storage/storage_backend_fs.c:849
  #5  storagePoolStart () at storage/storage_driver.c:700
  #6  virStoragePoolCreate () at libvirt.c:12471
  ...
 
 Treat files which miss their backing file as standalone files.

Makes sense; I don't know of anything better that we can do.

ACK and pushed.

 
 Signed-off-by: Philipp Hahn h...@univention.de
 ---
  src/util/storage_file.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/src/util/storage_file.c b/src/util/storage_file.c
 index e9771d7..2249212 100644
 --- a/src/util/storage_file.c
 +++ b/src/util/storage_file.c
 @@ -727,8 +727,9 @@ virStorageFileGetMetadataFromBuf(int format,
  meta-backingStoreRaw = meta-backingStore;
  meta-backingStore = absolutePathFromBaseFile(path, backing);
  if (meta-backingStore == NULL) {
 -VIR_FREE(backing);
 -return -1;
 +/* the backing file is (currently) unavailable, treat 
 this
 + * file as standalone */
 +backingFormat = VIR_STORAGE_FILE_NONE;
  }
  }
  VIR_FREE(backing);
 

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Gene Czarcinski]

On 11/08/2012 05:41 PM, Gene Czarcinski wrote:
The reult is a very private IPv6 network between the virtual guest 
systems.
A bit of clarification on why I would want such a capability (and, in 
truth, I have it today but I wanted to make it more automatic and 
available to anyone else).


Lets say that (hypothetically) we want to set up a firewall, dmz, 
whatever so that we can (hypothetically)do some attack testing against 
the systems.


To say the least (at least in the USA) this is very much frond upon on 
the real Internet.  So, set up a real heardware network ... this gets 
expensive real fast.


So, virtualization to the rescue.  Set up your network configuration on 
some very private networks (yes, they will need their own dns, dhcp, RA, 
etc., etc. services).


I can do it (and have) but I thought this might be useful to others.  
Obviously, this update should be accompanied by some documentation 
updates which explain what can be done.


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: Fix domain ID numbering race condition

[Peter Krempa]

On 11/08/12 17:21, Eric Blake wrote:

On 11/08/2012 06:09 AM, Peter Krempa wrote:

When the libvirt daemon is restarted it tries to reconnect to running
qemu domains. Since commit d38897a5d4b1880e1998394b2a37bba979bbdff1 the
re-connection code runs in separate threads. In the original
implementation the maximum of domain ID's (that is used as an
initializer for numbering guests created next) while libvirt was
reconnecting to the guest.




+/* find the maximum ID from active and transient configs to initialize
+ * the driver with. This is to avoid race between autostart and reconnect
+ * threads */
+virHashForEach(qemu_driver-domains.objs,
+   qemuDomainFindMaxID,
+   (qemu_driver-nextvmid));


Spurious parens; this can be just qemu_driver-nextvmid.

ACK.



I removed the extra parentheses and pushed the patch. Thanks for the review.

Peter

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] dnsmasq supporting RA instead of radvd patch

[Jim Fehlig]

Gene Czarcinski wrote:
  I have a working patch to have dnsmasq support RA instead of radvd. 
 However, something has come up and it will be a week to ten days
 before I can get it in shape to submit.

 The current patch has three variables added to the _virNetworkObj
 structure: dnsmasqRA flag and both major and minor values for the
 dnsmasq's version.

 I use dnsmasq --version and then parse out the major/minor version
 values.  If major2, then dnamsqFA=1.  If major=2 and minor=63, then
 dnsmasqRA=1.  For all other cases, dnsmasqRA=0.

 Code is added to the radvd functions which checks dnsmasqRA and exits
 if it is 1.

 Code is added to the dnsmasq configuration file if dnsmasqRa=1.  If
 dhcp-range or dhcp-hosts is specified for IPv6, then enable-ra is
 added for stateful (dhcpv6).  Otherwise, a special
 dhcp-range=ipv6-subnet-address,ra-only so that the ManagedFlag
 will be off in the RA packets for stateless operation.

 OK, how does that sound?  Everyone comfortable with that?

 Another thing is that I plan to add a test such that if the radvd
 executable is not valid, the dnsmasqRA=1.

 As I was doing this, I also looked through the libvirt.spec file. My,
 what a wonderful example of wizardly that is.  Anyway, I thought some
 updates may be in ortder:

 - increase the minimum version for dnsmasq from 2.41 to 2.48.

The SLE11 code stream, which will be active for quite some years, has
version 2.45 :(.  Why is 2.48 needed?  The feature you are after is in
2.63 right?

Regards,
Jim


 - why is radvd required for rpmbuild?

 - in light of my patch, make radvd an optional runtime requirement. I
 am not a spec file expert by any means but there must be a way to not
 require radvd if dnsmasq - 2.63.

 Comments?

 Gene

 -- 
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Expose virDomainGetAutostart through virsh?

[Guido Günther]

On Thu, Nov 08, 2012 at 03:42:49PM +0100, Daniel P. Berrange wrote:
 On Thu, Nov 08, 2012 at 03:41:12PM +0100, Peter Krempa wrote:
  On 11/08/12 15:13, Daniel P. Berrange wrote:
  On Wed, Nov 07, 2012 at 11:32:22AM +0100, Peter Krempa wrote:
  On 11/07/12 11:04, Ruben Kerkhof wrote:
  Hi list,
  
  I'd like to check if a domain has autostart enabled. I do this now by
  looking if there's a symlink in /etc/libvirt/qemu/autostart, but it
  feels a bit hackish.
  
  Is this something that could be added to virsh?
  Something like virsh get-autostart domain would be great.
  
  For now there are two options how to check the autostart flag:
  
  1) virsh dominfo - This is suitable to check for the state of a
  single guest. Unfortunately we have just this one output option
  where it is embedded with other information about the guest:
  
  $ virsh dominfo tr
  Id: 1
  Name:   tr
  UUID:   17f42b42-9fdd-81e3-4a93-a75021a707d3
  OS Type:hvm
  State:  running
  CPU(s): 1
  CPU time:   200.5s
  Max memory: 53248 KiB
  Used memory:53248 KiB
  Persistent: yes
  Autostart:  enable- here!
  Managed save:   no
  Security model: none
  Security DOI:   0
  
  
  2) use virsh list --all --autostart to list guests that have the
  autostart flag enabled (also note that there are script-friendly
  outputs for virsh list --name and --uuid):
  
  $ virsh list --all --autostart
IdName   State
  
1 tr running
- Bare   shut off
  
  
  We're planing on adding the autostart flag as an XML element and a
  few other improvements of autostaring guests.
  
  I don't think the XML should be in the business of having the
  autostart flag, as this is not guest configuration information.
  Having it in the XML introduces the problem of maintaining the
  correct synchronization between the autostart symlink and the
  XML description.
  
  Yes, that will be troublesome. It's unfortunate we still have to use
  the symlink approach.
  
  
  Further, in the future I'd like to actually replace our current
  autostart code, with code that just sets up systemd units to
  deal with autostart.
  
  In such a scenario, the user will be able to toggle autostart
  directly using systemd, so we don't want libvirt duplicating
  that info in the XML since it won't be aware of when systemd
  changes the autostart flag.
  
  Note that there are still distros that are doing everything possible
  to avoid having systemd. It wouldn't be nice if we broke
  autostarting there.
 
 Of course, not least RHEL-5 and RHEL-6. If we did support systemd,
 we would have a compile time option to use systemd vs our current
 autostart code.
Can we make this runtime then? Some distroy like Debian have systemd
optional so toggling this based on systemd actually being used would be
most helpful.
Cheers,
 -- Guido

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Laine Stump]

On 11/08/2012 04:44 PM, Daniel P. Berrange wrote:
 On Thu, Nov 08, 2012 at 02:41:29PM -0500, Laine Stump wrote:
 On 11/07/2012 04:25 PM, Gene Czarcinski wrote:
 IPv4 and IPv6 networks are suppose to have the same (more or less)
 functionality so why isn't this OK.
 Maintaining backward compatibility, both API and operational. In the
 past it wasn't the case that we simply did nothing about ipv6 on
 libvirt's networks, instead we explicitly set a sysctl to *disable* it.
 That must have been done for some reason. That reason may no longer be
 valid, but we don't know that yet (it happened before I was around). If
 the reason is no longer valid, we can go ahead as you suggest (and I
 would say we don't even need an option to not have ip6tables, just force
 people to build the full iptables package as God intended :-P). If the
 reason *is* still valid, then we need to only enable the ipv6 sysctl and
 add the ip6tables rule in response to some new flag attribute in the
 network config.
 If you don't disable IPv6 on the bridge device, then when starting the
 network device, the kernel will auto-assign an IPv6 link local address,
 which the guest can then use to communicate with the host. In the IPv4
 case, if you don't specify any ip address, there is no link local
 like address present, so there's no connectivity between guest and
 host. So explicitly disabling IPv6 is in fact required in order to
 give consistent behaviour between IPv6 and IPv4.

Okay, so there's the straight dope :-)

 I've no objections to anyone adding a new 'ipv6=on|off' attribute to
 the network XML so that admins can explicitly choosen whether to allow
 IPv6, indepedently of whether any ip element is set with an IPv6 address.

Hmm - would it maybe be okay to always add the ip6tables rule to allow
ipv6 traffic between interfaces on the bridge, while still setting
disable_ipv6=1 (unless there is an ip with an ipv6 address)? The
guests could then do IPv6 among themselves if they wanted, but there
would be no way to get to the host via IPv6.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Proposed: always allow packets internal to an interface

[Gene Czarcinski]

On 11/08/2012 09:01 PM, Laine Stump wrote:

Hmm - would it maybe be okay to always add the ip6tables rule to allow
ipv6 traffic between interfaces on the bridge, while still setting
disable_ipv6=1 (unless there is an ip with an ipv6 address)? The
guests could then do IPv6 among themselves if they wanted, but there
would be no way to get to the host via IPv6.

All I can say is that it seems to work ... at least my definition of work.

Obviously (I hope) the virtualization host sees nothing of the 
communications between the virtual systems on the very private virtual 
network.


Take a look at my message which describes what I did.  Give it a try for 
your self and tell us what you see.


I do not know how things are suppose to work, I can only report on how 
they do work.


Now, that is not to say that if you call some function to do something, 
that, in addition to performing what you want, it also does some other 
stuff.  That is, if I have ip6tables do something, is it also doing 
something else?


Gene

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/2] helper of copy-storage-* features

[li guang]

Hi, Eric
any comment on this patch?


在 2012-11-06二的 16:09 +0800，li guang写道：
 ping ...
 在 2012-11-01四的 08:40 +0800，liguang写道：
  help to create disk images copy-storage-* required,
  try to do non-shared migration without bothering to
  create disk images at target by hand.
  
  consider this situation:
  1. non-shared migration
 virsh migrate --copy-storage-all ...
  2. migration fails
  3. create disk images required
 qemu-img create ...
  4  migration run smoothly
  so, try do remove step 2, 3, 4
  
  this kind of usage had been discussed before,
  http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
  
  maybe there're some flaws:
  - It did not handle more about complete situations
suggested by Daniel P. Berrange,
https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html
but may try to take care of them later.
so, now only normal disk image files be handled.
  - for creation of disk images, size was setting as 0x boldly,
hope it can consolidate qemu, haven't constructed a comfortable
idea to solve it.
  
  v2:
1. handle disk encrytion case
2. check kvm-img  qemu-img
3. set disk image size to 0xfffK bytes blindly
  
  v3:
  1.use qemuImgBinary to create disk image
  2.set max size for different disk image format respectively
qcow and qcow2: 1PiB
qed:64TiB
raw:1TiB
from qemu's setting,
qcow and qcow2's max size is 2EiB,
qed's max size is 64TiB
raw's max size is 1TiB
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   src/qemu/qemu_migration.c |  122 
  -
   1 files changed, 120 insertions(+), 2 deletions(-)
  
  diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
  index db69a0a..80abb51 100644
  --- a/src/qemu/qemu_migration.c
  +++ b/src/qemu/qemu_migration.c
  @@ -49,6 +49,7 @@
   #include storage_file.h
   #include viruri.h
   #include hooks.h
  +#include dirname.h
   
  
   #define VIR_FROM_THIS VIR_FROM_QEMU
  @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags {
   QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
   QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
   QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
  +QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
   
   QEMU_MIGRATION_COOKIE_FLAG_LAST
   };
  @@ -77,12 +79,13 @@ enum qemuMigrationCookieFlags {
   VIR_ENUM_DECL(qemuMigrationCookieFlag);
   VIR_ENUM_IMPL(qemuMigrationCookieFlag,
 QEMU_MIGRATION_COOKIE_FLAG_LAST,
  -  graphics, lockstate, persistent);
  +  graphics, lockstate, persistent, copystorage);
   
   enum qemuMigrationCookieFeatures {
   QEMU_MIGRATION_COOKIE_GRAPHICS  = (1  
  QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
   QEMU_MIGRATION_COOKIE_LOCKSTATE = (1  
  QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
   QEMU_MIGRATION_COOKIE_PERSISTENT = (1  
  QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
  +QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1  
  QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE),
   };
   
   typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
  @@ -439,6 +442,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver 
  *driver,
   virBufferAdjustIndent(buf, -2);
   }
   
  +if (mig-flags  QEMU_MIGRATION_COOKIE_COPYSTORAGE)
  +virBufferAsprintf(buf,   copystorage/\n);
  +
   virBufferAddLit(buf, /qemu-migration\n);
   return 0;
   }
  @@ -662,6 +668,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
   VIR_FREE(nodes);
   }
   
  +if ((flags  QEMU_MIGRATION_COOKIE_COPYSTORAGE)) {
  +if (virXPathBoolean(count(./copystorage)  0, ctxt))
  +mig-flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE;
  +}
  +
   return 0;
   
   error:
  @@ -721,6 +732,9 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
   qemuMigrationCookieAddPersistent(mig, dom)  0)
   return -1;
   
  +if (flags  QEMU_MIGRATION_COOKIE_COPYSTORAGE)
  +mig-flags |= QEMU_MIGRATION_COOKIE_COPYSTORAGE;
  +
   if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
   return -1;
   
  @@ -1168,6 +1182,14 @@ char *qemuMigrationBegin(struct qemud_driver *driver,
   QEMU_MIGRATION_COOKIE_LOCKSTATE)  0)
   goto cleanup;
   
  +if (flags  (VIR_MIGRATE_NON_SHARED_DISK |
  + VIR_MIGRATE_NON_SHARED_INC)) {
  +if (qemuMigrationBakeCookie(mig, driver, vm,
  +cookieout, cookieoutlen,
  +QEMU_MIGRATION_COOKIE_COPYSTORAGE)  0)
  +goto cleanup;
  +}
  +
   if (xmlin) {
   if (!(def = virDomainDefParseString(driver-caps, xmlin,
   QEMU_EXPECTED_VIRT_TYPES,
  @@ -1215,6 +1237,89 @@ qemuMigrationPrepareCleanup(struct qemud_driver 
  *driver,
   qemuDomainObjDiscardAsyncJob(driver, vm);
   }
   
  +/*
  +  if gen_del is 

Re: [libvirt] [PATCH v13] support offline migration

[li guang]

在 2012-11-09五的 08:44 +0800，li guang写道：
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index 978af57..6c2bf98 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -9796,7 +9796,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
   asyncJob = QEMU_ASYNC_JOB_NONE;
   }
   
  -if (!virDomainObjIsActive(vm)) {
  +if (!virDomainObjIsActive(vm)  !(flags  VIR_MIGRATE_OFFLINE))
 {
   virReportError(VIR_ERR_OPERATION_INVALID,
  %s, _(domain is not running));
   goto endjob;
  @@ -9805,9 +9805,9 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
   /* Check if there is any ejected media.
* We don't want to require them on the destination.
*/
  -
  -if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob)  0)
  -goto endjob;
  +if (virDomainObjIsActive(vm))
  +if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob)  0)
  +goto endjob;
   
   if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname,
  cookieout, cookieoutlen,
 
 What if you want to offline migrate a domain that is currently running?
 I think it's better to first move qemuDomainCheckEjectableMedia down to
 qemuMigrationBegin in a separate patch and then just use the code from
 v12 (without using the offline label).

sorry, I don't like to do offline migrate for a domain that's running.

 
  diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
  index 5f8a9c5..66fbc02 100644
  --- a/src/qemu/qemu_migration.c
  +++ b/src/qemu/qemu_migration.c
  @@ -72,6 +72,7 @@ enum qemuMigrationCookieFlags {
   QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
   QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
   QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
  +QEMU_MIGRATION_COOKIE_FLAG_OFFLINE,
   
   QEMU_MIGRATION_COOKIE_FLAG_LAST
   };
  @@ -79,13 +80,14 @@ enum qemuMigrationCookieFlags {
   VIR_ENUM_DECL(qemuMigrationCookieFlag);
   VIR_ENUM_IMPL(qemuMigrationCookieFlag,
 QEMU_MIGRATION_COOKIE_FLAG_LAST,
  -  graphics, lockstate, persistent, network);
  +  graphics, lockstate, persistent, network,
 offline);
   
   enum qemuMigrationCookieFeatures {
   QEMU_MIGRATION_COOKIE_GRAPHICS  = (1 
 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
   QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 
 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
   QEMU_MIGRATION_COOKIE_PERSISTENT = (1 
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
   QEMU_MIGRATION_COOKIE_NETWORK = (1 
 QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
  +QEMU_MIGRATION_COOKIE_OFFLINE = (1 
 QEMU_MIGRATION_COOKIE_FLAG_OFFLINE),
   };
   
   typedef struct _qemuMigrationCookieGraphics
 qemuMigrationCookieGraphics;
  @@ -594,6 +596,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver
 *driver,
   if ((mig-flags  QEMU_MIGRATION_COOKIE_NETWORK)  mig-network)
   qemuMigrationCookieNetworkXMLFormat(buf, mig-network);
   
  +if (mig-flags  QEMU_MIGRATION_COOKIE_OFFLINE)
  +virBufferAsprintf(buf,   offline/\n);
  +
   virBufferAddLit(buf, /qemu-migration\n);
   return 0;
   }
  @@ -874,6 +879,11 @@
 qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
   (!(mig-network = qemuMigrationCookieNetworkXMLParse(ctxt
   goto error;
   
  +if ((flags  QEMU_MIGRATION_COOKIE_OFFLINE)) {
  +if (virXPathBoolean(count(./offline)  0, ctxt))
  +mig-flags |= QEMU_MIGRATION_COOKIE_OFFLINE;
  +}
  +
   return 0;
   
   error:
  @@ -938,6 +948,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr
 mig,
   return -1;
   }
   
  +if (flags  QEMU_MIGRATION_COOKIE_OFFLINE) {
  +mig-flags |= QEMU_MIGRATION_COOKIE_OFFLINE;
  +}
  +
   if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
   return -1;
   
 
 Oh, I'm sorry for not noticing this earlier, but why exactly do we need
 this offline/ element in migration cookie? It doesn't look like we need to
 store some additional data required for offline migration. I think just
 passing the VIR_MIGRATE_OFFLINE flag will be better. Not to mention that if 
 an older
 libvirt gets a cookie with offline/ element, it will just ignore it
 while passing a flag (which the code should already been doing anyway) should
 make it fail for unsupported flag.

without this how do you know you a offline migration at target side?

 
  @@ -1443,6 +1457,24 @@ char *qemuMigrationBegin(struct qemud_driver
 *driver,
   QEMU_MIGRATION_COOKIE_LOCKSTATE)  0)
   goto cleanup;
   
  +if (flags  VIR_MIGRATE_OFFLINE) {
  +if (flags  (VIR_MIGRATE_NON_SHARED_DISK|
  + VIR_MIGRATE_NON_SHARED_INC)) {
  +virReportError(VIR_ERR_OPERATION_INVALID,
  +   %s, _(offline migration cannot handle
 non-shared storage));
  +goto cleanup;
  +}
  +if (!(flags  

Re: [libvirt] [PATCH v3 2/2] init qemu_driver's qemuImgBinary field

[li guang]

Hi, Eric
any comment on this patch?

在 2012-11-06二的 16:09 +0800，li guang写道：
 ping ...
 
 在 2012-11-01四的 08:40 +0800，liguang写道：
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   src/qemu/qemu_domain.c |2 +-
   src/qemu/qemu_driver.c |3 +++
   2 files changed, 4 insertions(+), 1 deletions(-)
  
  diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
  index 17ae3b9..ac16772 100644
  --- a/src/qemu/qemu_domain.c
  +++ b/src/qemu/qemu_domain.c
  @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver 
  *driver,
   int i;
   bool skipped = false;
   
  -qemuimgarg[0] = qemuFindQemuImgBinary(driver);
  +qemuimgarg[0] = driver-qemuImgBinary;
   if (qemuimgarg[0] == NULL) {
   /* qemuFindQemuImgBinary set the error */
   return -1;
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index 97ad23e..882e95a 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -624,6 +624,9 @@ qemudStartup(int privileged) {
   if (!qemu_driver-domainEventState)
   goto error;
   
  +/*find kvm-img or qemu-img*/
  +qemuFindQemuImgBinary(qemu_driver);
  +
   /* read the host sysinfo */
   if (privileged)
   qemu_driver-hostsysinfo = virSysinfoRead();
 

-- 
li guang  lig.f...@cn.fujitsu.com
linux kernel team at FNST, china


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[Eric Blake]

On 11/08/2012 07:57 PM, li guang wrote:
 What if you want to offline migrate a domain that is currently running?
 I think it's better to first move qemuDomainCheckEjectableMedia down to
 qemuMigrationBegin in a separate patch and then just use the code from
 v12 (without using the offline label).
 
 sorry, I don't like to do offline migrate for a domain that's running.

Why not, when we've already argued it can be useful?  You don't have to
support it in the same patch, but you should at least take the advice on
how to refactor things so that someone else that does like the idea can
extend things to provide it.  There's nothing wrong with having
persistent definitions of the same domain on more than one machine, as
long as only one machine at a time is running it; and after all, with
live migration, if you _don't_ pass the --undefinesource flag, you will
be left in the same situation where a persistent domain on the source is
left behind even when the running domain has migrated.


 Oh, I'm sorry for not noticing this earlier, but why exactly do we need
 this offline/ element in migration cookie? It doesn't look like we need to
 store some additional data required for offline migration. I think just
 passing the VIR_MIGRATE_OFFLINE flag will be better. Not to mention that if 
 an older
 libvirt gets a cookie with offline/ element, it will just ignore it
 while passing a flag (which the code should already been doing anyway) should
 make it fail for unsupported flag.
 
 without this how do you know you a offline migration at target side?

By the presence or absence of the flag.  If the flag is present, you are
doing an offline migration.  I agree that we don't need to add anything
to the cookie, since the only way to get offline migration is via the
new flag, and both source and destination will see the same set of flags.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[li guang]

在 2012-11-08四的 20:28 -0700，Eric Blake写道：
 On 11/08/2012 07:57 PM, li guang wrote:
  What if you want to offline migrate a domain that is currently running?
  I think it's better to first move qemuDomainCheckEjectableMedia down to
  qemuMigrationBegin in a separate patch and then just use the code from
  v12 (without using the offline label).
  
  sorry, I don't like to do offline migrate for a domain that's running.
 
 Why not, when we've already argued it can be useful?  You don't have to
 support it in the same patch, but you should at least take the advice on
 how to refactor things so that someone else that does like the idea can
 extend things to provide it.  There's nothing wrong with having
 persistent definitions of the same domain on more than one machine, as
 long as only one machine at a time is running it; and after all, with
 live migration, if you _don't_ pass the --undefinesource flag, you will
 be left in the same situation where a persistent domain on the source is
 left behind even when the running domain has migrated.

I mean we should do this explicitly, e.g. an flag to notify us we are 
doing job for active domain by walking through offline path.
I said this before, at comment on virsh change.

 
 
  Oh, I'm sorry for not noticing this earlier, but why exactly do we need
  this offline/ element in migration cookie? It doesn't look like we need 
  to
  store some additional data required for offline migration. I think just
  passing the VIR_MIGRATE_OFFLINE flag will be better. Not to mention that 
  if an older
  libvirt gets a cookie with offline/ element, it will just ignore it
  while passing a flag (which the code should already been doing anyway) 
  should
  make it fail for unsupported flag.
  
  without this how do you know you a offline migration at target side?
 
 By the presence or absence of the flag.  If the flag is present, you are

seem flags was forgotten at qemuDomainMigratePrepare3, so any
preparation for offline migration will be impossible, isn't it?
do you mean I should change MigratePrepare functions to add
flags as a parameter?

 doing an offline migration.  I agree that we don't need to add anything
 to the cookie, since the only way to get offline migration is via the
 new flag, and both source and destination will see the same set of flags.
 

-- 
li guang  lig.f...@cn.fujitsu.com
linux kernel team at FNST, china


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[Eric Blake]

On 11/08/2012 08:54 PM, li guang wrote:
 在 2012-11-08四的 20:28 -0700，Eric Blake写道：
 On 11/08/2012 07:57 PM, li guang wrote:
 What if you want to offline migrate a domain that is currently running?
 I think it's better to first move qemuDomainCheckEjectableMedia down to
 qemuMigrationBegin in a separate patch and then just use the code from
 v12 (without using the offline label).

 sorry, I don't like to do offline migrate for a domain that's running.

 Why not, when we've already argued it can be useful?  You don't have to
 support it in the same patch, but you should at least take the advice on
 how to refactor things so that someone else that does like the idea can
 extend things to provide it.  There's nothing wrong with having
 persistent definitions of the same domain on more than one machine, as
 long as only one machine at a time is running it; and after all, with
 live migration, if you _don't_ pass the --undefinesource flag, you will
 be left in the same situation where a persistent domain on the source is
 left behind even when the running domain has migrated.
 
 I mean we should do this explicitly, e.g. an flag to notify us we are 
 doing job for active domain by walking through offline path.
 I said this before, at comment on virsh change.

You ARE being explicit when you say 'virsh migrate --offline'.  If the
domain is transient, it must fail, but if the domain is persistent, then
whether or not it is running, you are requesting that ONLY the offline
state be migrated.


 without this how do you know you a offline migration at target side?

 By the presence or absence of the flag.  If the flag is present, you are
 
 seem flags was forgotten at qemuDomainMigratePrepare3, so any
 preparation for offline migration will be impossible, isn't it?
 do you mean I should change MigratePrepare functions to add
 flags as a parameter?

Huh?  qemuDomainMigratePrepare3 already has a flags argument (arg 8),
and it is directly related to the flags argument that was originally
passed to virDomainMigrate() in libvirt.c.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v13] support offline migration

[li guang]

在 2012-11-08四的 21:06 -0700，Eric Blake写道：
 On 11/08/2012 08:54 PM, li guang wrote:
  在 2012-11-08四的 20:28 -0700，Eric Blake写道：
  On 11/08/2012 07:57 PM, li guang wrote:
  What if you want to offline migrate a domain that is currently running?
  I think it's better to first move qemuDomainCheckEjectableMedia down to
  qemuMigrationBegin in a separate patch and then just use the code from
  v12 (without using the offline label).
 
  sorry, I don't like to do offline migrate for a domain that's running.
 
  Why not, when we've already argued it can be useful?  You don't have to
  support it in the same patch, but you should at least take the advice on
  how to refactor things so that someone else that does like the idea can
  extend things to provide it.  There's nothing wrong with having
  persistent definitions of the same domain on more than one machine, as
  long as only one machine at a time is running it; and after all, with
  live migration, if you _don't_ pass the --undefinesource flag, you will
  be left in the same situation where a persistent domain on the source is
  left behind even when the running domain has migrated.
  
  I mean we should do this explicitly, e.g. an flag to notify us we are 
  doing job for active domain by walking through offline path.
  I said this before, at comment on virsh change.
 
 You ARE being explicit when you say 'virsh migrate --offline'.  If the
 domain is transient, it must fail, but if the domain is persistent, then
 whether or not it is running, you are requesting that ONLY the offline
 state be migrated.

reasonable? maybe.

 
 
  without this how do you know you a offline migration at target side?
 
  By the presence or absence of the flag.  If the flag is present, you are
  
  seem flags was forgotten at qemuDomainMigratePrepare3, so any
  preparation for offline migration will be impossible, isn't it?
  do you mean I should change MigratePrepare functions to add
  flags as a parameter?
 
 Huh?  qemuDomainMigratePrepare3 already has a flags argument (arg 8),
 and it is directly related to the flags argument that was originally
 passed to virDomainMigrate() in libvirt.c.
 

please check qemuMigrationPrepareDirect called in
qemuDomainMigratePrepare3

-- 
li guang  lig.f...@cn.fujitsu.com
linux kernel team at FNST, china


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [RFC PATCH 1/n] snapshot: add revert-and-create branching of external snapshots

[Eric Blake]

Right now, libvirt refuses to revert to the state at the time
of an external snapshot, because doing so would reset things so
that the next time the domain boots, we are using the backing
file; but modifying the backing file invalidates all qcow2
files that are based on top of it.  There are three possibilities
for lifting this restriction:
1. delete all snapshot metadata and qcow2 files that are
invalidated by the revert (losing changes since the snapshot)
2. perform a block commit (such as with qemu-img commit) to
merge the qcow2 file back into the backing file (keeping the
changes since the snapshot)
3. create NEW qcow2 files that wrap the same base file as
the original snapshot (keeping the changes since the original
snapshot)

This patch documents the API for option 3, by adding a new flag
to virDomainSnapshotCreateXML (the only snapshot-related function
that takes XML, which is required to pass in new file names
during the branch), and wires up virsh to expose it.  Later
patches will enhance virDomainRevertToSnapshot to cover options
1 and 2.

API wise, an application wishing to do the revert-and-create
operation must add a branchname/branch element to their
domainsnapshot XML, and pass VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH
in the flags argument.  In virsh, snapshot-create gains a new
boolean flag --branch that must match the XML passed in, and
snapshot-create-as gains a new --branch=name argument along
with a --current boolean for creating such XML.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH): New flag.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it, and
enforce some mutual exclusion.
(virDomainRevertToSnapshot): Mention how to revert to an
external snapshot without having to delete snapshots.
* docs/formatsnapshot.html.in: Document the new branch element.
* docs/schemas/domainsnapshot.rng: Allow new element.
* tools/virsh-snapshot.c (cmdSnapshotCreate): Add --branch option.
(cmdSnapshotCreateAs): Likewise, also add --current.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
new usage.
---

Sending this patch now, to make sure I'm on the right track.  I
have the following plans for the next few patches:

1.
Enhance virDomainSnapshotListFlags to add two new filter groups:
VIR_DOMAIN_SNAPSHOT_LIST_OFFLINE
VIR_DOMAIN_SNAPSHOT_LIST_ONLINE
VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY

VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL
VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL
(and possibly VIR_DOMAIN_SNAPSHOT_LIST_MIXED if we change our stance
and allow mixing internal and external in one snapshot)

2.
Add a flag to virDomainSnapshotParseFlags in src/conf/snapshot_conf.h;
when present, the new element is parsed, and appropriate elements of
the in-memory snapshot representation are copied in from the existing
external snapshot.

3.
Implement the new flag in qemu_driver.c for creation.  Note that
branching works for both disk-only (whether offline or online) and
checkpoints - the new snapshot will share the same memory (none or
external) as the original, and all that really changes is the
creation (or reuse) of new backing files.

4.
Think about ramifications on revert - right now, with no options,
revert means to go back to the state where the snapshot was created;
but now that we allow the creation of branches, and since the branches
have external disk state which persists even when not on the branch, we
need a new revert flag that says to revert to an external snapshots
and its subsequent changes

5.
Think about ramifications on delete - for example, if two branches
of external snapshots share a common memory state file, and we delete
only one of the two branches, the memory state file must not be
deleted.

 docs/formatsnapshot.html.in | 23 +++
 docs/schemas/domainsnapshot.rng |  5 +
 include/libvirt/libvirt.h.in|  3 +++
 src/libvirt.c   | 35 ++-
 tools/virsh-snapshot.c  | 31 ++-
 tools/virsh.pod | 16 ++--
 6 files changed, 97 insertions(+), 16 deletions(-)

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 8fcc04c..5018f41 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -87,7 +87,12 @@
   sets that snapshot as current, and the prior current snapshot is
   the parent of the new snapshot.  Branches in the hierarchy can
   be formed by reverting to a snapshot with a child, then creating
-  another snapshot.
+  another snapshot.  In the case of external snapshots, modifying
+  the backing files would invalidate all external files that
+  depend on the backing file, so the action of reverting to a
+  snapshot must be accompanied by either a request to delete all
+  invalidated snapshots, or to create a new snapshot at the same
+  time as the revert.
 /p
 p
   The top-level codedomainsnapshot/code element may contain
@@ -188,9 +193,19 @@
   

Re: [libvirt] [PATCH v13] support offline migration

[Eric Blake]

On 11/08/2012 09:31 PM, li guang wrote:
 seem flags was forgotten at qemuDomainMigratePrepare3, so any
 preparation for offline migration will be impossible, isn't it?
 do you mean I should change MigratePrepare functions to add
 flags as a parameter?

 Huh?  qemuDomainMigratePrepare3 already has a flags argument (arg 8),
 and it is directly related to the flags argument that was originally
 passed to virDomainMigrate() in libvirt.c.

 
 please check qemuMigrationPrepareDirect called in
 qemuDomainMigratePrepare3

qemuMigrationPrepareDirect is not a callback, and not a public API, so
you are free to change its signature and add a flags parameter if that
is necessary to get the information to the right places.  The only
signatures you can't change are in src/libvirt.c and in functions
serving as callbacks registered through signatures in src/driver.h.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/2] helper of copy-storage-* features

[Eric Blake]

On 10/31/2012 06:40 PM, liguang wrote:
 help to create disk images copy-storage-* required,
 try to do non-shared migration without bothering to
 create disk images at target by hand.
 
 consider this situation:
 1. non-shared migration
virsh migrate --copy-storage-all ...
 2. migration fails
 3. create disk images required
qemu-img create ...
 4  migration run smoothly
 so, try do remove step 2, 3, 4
 
 this kind of usage had been discussed before,
 http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html

Typically, you would put your signed off line here, followed by ---, so
that the rest of this commentary...

 
 maybe there're some flaws:
 - It did not handle more about complete situations
   suggested by Daniel P. Berrange,
   https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html
   but may try to take care of them later.
   so, now only normal disk image files be handled.
 - for creation of disk images, size was setting as 0x boldly,

Setting to MAX_INT sounds wrong.  Either you create the destination
image empty and then let qemu automatically enlarge it as it populates
incoming data (preferred), or you need to pass size information in the
cookie (harder).

   hope it can consolidate qemu, haven't constructed a comfortable
   idea to solve it.
 
 v2:
   1. handle disk encrytion case
   2. check kvm-img  qemu-img
   3. set disk image size to 0xfffK bytes blindly
 
 v3:
 1.use qemuImgBinary to create disk image
 2.set max size for different disk image format respectively
   qcow and qcow2: 1PiB
   qed:64TiB
   raw:1TiB
   from qemu's setting,
   qcow and qcow2's max size is 2EiB,
   qed's max size is 64TiB
   raw's max size is 1TiB

...remains useful for reviewing on list but is not codified in git when
the patch is finally approved.  That is, when reading 'git log', we
don't care how many versions it took us to get to the right solution,
only that the right solution is in git.  'git am' automatically strips
comments after the first ---, making it a useful divider between the
commit description and the notes for reviewers.

 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/qemu/qemu_migration.c |  122 
 -
  1 files changed, 120 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index db69a0a..80abb51 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -49,6 +49,7 @@
  #include storage_file.h
  #include viruri.h
  #include hooks.h
 +#include dirname.h
  
  
  #define VIR_FROM_THIS VIR_FROM_QEMU
 @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags {
  QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
  QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
  QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
 +QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
  

I'm not yet convinced whether you need to do anything with the cookie.
Rather, it should be sufficient to add a new public VIR_MIGRATE_* flag
that says whether libvirt should be attempting to create destination
files when given VIR_MIGRATE_NON_SHARED_{DISK,INC}.  Then, just like the
offline migration case, all you have to do is make sure the flag is
properly passed through the entire migration chain to all the points
that care about it.  But if you DO need the cookie, the I think you need
more than just a single flag - you need to pass a list of all file
information (such as size) that will be needed to create the same types
of files on the destination.

  
 +/*
 +  if gen_del is 1, find out disk images migration required,
 +  so try to generate them at target,
 +  if gen_del is 0, delete disk images generated before.
 +*/
 +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver,
 +virDomainDefPtr def, int gen_del)

It sounds like you are using gen_del as a bool - if so, type it as bool,
not int.  And its name is confusing; I might go with 'bool generate',
where true means generate, and false means delete.

 +{
 +char *tmp_dir = NULL, *outbuf = NULL;
 +char *img_tool = driver-qemuImgBinary;

Don't grab this field directly.  Instead, use qemuFindQemuImgBinary(driver).

 +virCommandPtr cmd = NULL;
 +int i, ret = -1;
 +
 +if (!def-ndisks)
 +return 0;
 +
 +if (img_tool == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   %s, _(unable to find kvm-img or qemu-img));
 +goto error;
 +}
 +
 +for (i = 0; i  def-ndisks; i++) {
 +if (STRNEQ(def-disks[i]-driverName, qemu))
 +continue;

You need to rebase your patches on top of the latest libvirt.git.  The
driverName field no longer exists; it is now an enum value named 'format'.

 +if (def-disks[i]-src == NULL)
 +continue;
 +if (def-disks[i]-driverType == NULL)
 +continue;
 +if (virFileExists(def-disks[i]-src)  gen_del)
 +continue;
 +if (!gen_del  

Re: [libvirt] [PATCH v13] support offline migration

[li guang]

在 2012-11-08四的 21:50 -0700，Eric Blake写道：
 On 11/08/2012 09:31 PM, li guang wrote:
  seem flags was forgotten at qemuDomainMigratePrepare3, so any
  preparation for offline migration will be impossible, isn't it?
  do you mean I should change MigratePrepare functions to add
  flags as a parameter?
 
  Huh?  qemuDomainMigratePrepare3 already has a flags argument (arg 8),
  and it is directly related to the flags argument that was originally
  passed to virDomainMigrate() in libvirt.c.
 
  
  please check qemuMigrationPrepareDirect called in
  qemuDomainMigratePrepare3
 
 qemuMigrationPrepareDirect is not a callback, and not a public API, so
 you are free to change its signature and add a flags parameter if that
 is necessary to get the information to the right places.  The only
 signatures you can't change are in src/libvirt.c and in functions
 serving as callbacks registered through signatures in src/driver.h.
 

OK, will change.

-- 
li guang  lig.f...@cn.fujitsu.com
linux kernel team at FNST, china


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 2/2] init qemu_driver's qemuImgBinary field

[Eric Blake]

On 10/31/2012 06:40 PM, liguang wrote:
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/qemu/qemu_domain.c |2 +-
  src/qemu/qemu_driver.c |3 +++
  2 files changed, 4 insertions(+), 1 deletions(-)
 
 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
 index 17ae3b9..ac16772 100644
 --- a/src/qemu/qemu_domain.c
 +++ b/src/qemu/qemu_domain.c
 @@ -1654,7 +1654,7 @@ qemuDomainSnapshotForEachQcow2Raw(struct qemud_driver 
 *driver,
  int i;
  bool skipped = false;
  
 -qemuimgarg[0] = qemuFindQemuImgBinary(driver);
 +qemuimgarg[0] = driver-qemuImgBinary;

Personally, I like going through accessor methods rather than direct
field access, as it leaves us more freedom for changing the
implementation in the future.  I'm not sure this patch buys us anything,
other than it would fail to start the libvirtd qemu driver at startup
rather than on the first command that needed to use qemu-img. Since
failing early on a bad setup is generally useful, I guess I could live
with this patch, except that you need to rebase it on top of Peter's
recent patches that added another caller of qemuFindQemuImgBinary.  For
that matter, if you are going to gaurantee that driver-qemuImgBinary is
always set, it might be better to remove the qemuFindQemuImgBinary()
function altogether, and instead inline its body...

 +++ b/src/qemu/qemu_driver.c
 @@ -624,6 +624,9 @@ qemudStartup(int privileged) {
  if (!qemu_driver-domainEventState)
  goto error;
  
 +/*find kvm-img or qemu-img*/
 +qemuFindQemuImgBinary(qemu_driver);
 +

into this one remaining call site.

Also, I would rebase the series to put this patch first, not last, since
your patch 1/2 was using driver-qemuImgBinary instead of
qemuFindQemuImgBinary().

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 1/2] helper of copy-storage-* features

[li guang]

在 2012-11-08四的 22:10 -0700，Eric Blake写道：
 On 10/31/2012 06:40 PM, liguang wrote:
  help to create disk images copy-storage-* required,
  try to do non-shared migration without bothering to
  create disk images at target by hand.
  
  consider this situation:
  1. non-shared migration
 virsh migrate --copy-storage-all ...
  2. migration fails
  3. create disk images required
 qemu-img create ...
  4  migration run smoothly
  so, try do remove step 2, 3, 4
  
  this kind of usage had been discussed before,
  http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
 
 Typically, you would put your signed off line here, followed by ---, so
 that the rest of this commentary...
 

OK, got it.

  
  maybe there're some flaws:
  - It did not handle more about complete situations
suggested by Daniel P. Berrange,
https://www.redhat.com/archives/libvir-list/2012-October/msg00407.html
but may try to take care of them later.
so, now only normal disk image files be handled.
  - for creation of disk images, size was setting as 0x boldly,
 
 Setting to MAX_INT sounds wrong.  Either you create the destination
 image empty and then let qemu automatically enlarge it as it populates
 incoming data (preferred), or you need to pass size information in the
 cookie (harder).

can qemu automatically enlarge?
I tried, but failed.

 
hope it can consolidate qemu, haven't constructed a comfortable
idea to solve it.
  
  v2:
1. handle disk encrytion case
2. check kvm-img  qemu-img
3. set disk image size to 0xfffK bytes blindly
  
  v3:
  1.use qemuImgBinary to create disk image
  2.set max size for different disk image format respectively
qcow and qcow2: 1PiB
qed:64TiB
raw:1TiB
from qemu's setting,
qcow and qcow2's max size is 2EiB,
qed's max size is 64TiB
raw's max size is 1TiB
 
 ...remains useful for reviewing on list but is not codified in git when
 the patch is finally approved.  That is, when reading 'git log', we
 don't care how many versions it took us to get to the right solution,
 only that the right solution is in git.  'git am' automatically strips
 comments after the first ---, making it a useful divider between the
 commit description and the notes for reviewers.
 
  
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   src/qemu/qemu_migration.c |  122 
  -
   1 files changed, 120 insertions(+), 2 deletions(-)
  
  diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
  index db69a0a..80abb51 100644
  --- a/src/qemu/qemu_migration.c
  +++ b/src/qemu/qemu_migration.c
  @@ -49,6 +49,7 @@
   #include storage_file.h
   #include viruri.h
   #include hooks.h
  +#include dirname.h
   
   
   #define VIR_FROM_THIS VIR_FROM_QEMU
  @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags {
   QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
   QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
   QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
  +QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
   
 
 I'm not yet convinced whether you need to do anything with the cookie.
 Rather, it should be sufficient to add a new public VIR_MIGRATE_* flag
 that says whether libvirt should be attempting to create destination
 files when given VIR_MIGRATE_NON_SHARED_{DISK,INC}.  Then, just like the
 offline migration case, all you have to do is make sure the flag is
 properly passed through the entire migration chain to all the points
 that care about it.  But if you DO need the cookie, the I think you need
 more than just a single flag - you need to pass a list of all file
 information (such as size) that will be needed to create the same types
 of files on the destination.
 

as we talk before.

   
  +/*
  +  if gen_del is 1, find out disk images migration required,
  +  so try to generate them at target,
  +  if gen_del is 0, delete disk images generated before.
  +*/
  +static int qemuMigrationHandleDiskFiles(struct qemud_driver *driver,
  +virDomainDefPtr def, int gen_del)
 
 It sounds like you are using gen_del as a bool - if so, type it as bool,
 not int.  And its name is confusing; I might go with 'bool generate',
 where true means generate, and false means delete.

right, will fix.

 
  +{
  +char *tmp_dir = NULL, *outbuf = NULL;
  +char *img_tool = driver-qemuImgBinary;
 
 Don't grab this field directly.  Instead, use qemuFindQemuImgBinary(driver).

driver-qemuImgBinary is useful, I've initiated it.
why don't we use it?

 
  +virCommandPtr cmd = NULL;
  +int i, ret = -1;
  +
  +if (!def-ndisks)
  +return 0;
  +
  +if (img_tool == NULL) {
  +virReportError(VIR_ERR_INTERNAL_ERROR,
  +   %s, _(unable to find kvm-img or qemu-img));
  +goto error;
  +}
  +
  +for (i = 0; i  def-ndisks; i++) {
  +if (STRNEQ(def-disks[i]-driverName, qemu))
  +continue;
 
 You need to rebase your patches 

Re: [libvirt] Expose virDomainGetAutostart through virsh?

[Daniel P. Berrange]

On Fri, Nov 09, 2012 at 01:19:01AM +0100, Guido Günther wrote:
 On Thu, Nov 08, 2012 at 03:42:49PM +0100, Daniel P. Berrange wrote:
  On Thu, Nov 08, 2012 at 03:41:12PM +0100, Peter Krempa wrote:
   On 11/08/12 15:13, Daniel P. Berrange wrote:
   On Wed, Nov 07, 2012 at 11:32:22AM +0100, Peter Krempa wrote:
   On 11/07/12 11:04, Ruben Kerkhof wrote:
   Hi list,
   
   I'd like to check if a domain has autostart enabled. I do this now by
   looking if there's a symlink in /etc/libvirt/qemu/autostart, but it
   feels a bit hackish.
   
   Is this something that could be added to virsh?
   Something like virsh get-autostart domain would be great.
   
   For now there are two options how to check the autostart flag:
   
   1) virsh dominfo - This is suitable to check for the state of a
   single guest. Unfortunately we have just this one output option
   where it is embedded with other information about the guest:
   
   $ virsh dominfo tr
   Id: 1
   Name:   tr
   UUID:   17f42b42-9fdd-81e3-4a93-a75021a707d3
   OS Type:hvm
   State:  running
   CPU(s): 1
   CPU time:   200.5s
   Max memory: 53248 KiB
   Used memory:53248 KiB
   Persistent: yes
   Autostart:  enable  - here!
   Managed save:   no
   Security model: none
   Security DOI:   0
   
   
   2) use virsh list --all --autostart to list guests that have the
   autostart flag enabled (also note that there are script-friendly
   outputs for virsh list --name and --uuid):
   
   $ virsh list --all --autostart
 IdName   State
   
 1 tr running
 - Bare   shut off
   
   
   We're planing on adding the autostart flag as an XML element and a
   few other improvements of autostaring guests.
   
   I don't think the XML should be in the business of having the
   autostart flag, as this is not guest configuration information.
   Having it in the XML introduces the problem of maintaining the
   correct synchronization between the autostart symlink and the
   XML description.
   
   Yes, that will be troublesome. It's unfortunate we still have to use
   the symlink approach.
   
   
   Further, in the future I'd like to actually replace our current
   autostart code, with code that just sets up systemd units to
   deal with autostart.
   
   In such a scenario, the user will be able to toggle autostart
   directly using systemd, so we don't want libvirt duplicating
   that info in the XML since it won't be aware of when systemd
   changes the autostart flag.
   
   Note that there are still distros that are doing everything possible
   to avoid having systemd. It wouldn't be nice if we broke
   autostarting there.
  
  Of course, not least RHEL-5 and RHEL-6. If we did support systemd,
  we would have a compile time option to use systemd vs our current
  autostart code.
 Can we make this runtime then? Some distroy like Debian have systemd
 optional so toggling this based on systemd actually being used would be
 most helpful.

Sure, that's possible too.

FYI, i have no plans to work on actually implmenting this feature in the
near future - just wanted to let people know about the idea.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list