Re: [libvirt] [PATCH v2] virsh: Print error if specified bandwidth is invalid for blockjob

[Osier Yang]

于 2011年08月22日 21:52, Eric Blake 写道:

It's strange that the command fails but without any error if one
specifies as not a number. 

Pushed.

Osier

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

Re: [libvirt] [PATCH] qemu: Substitute VIR_ERR_NO_SUPPORT with VIR_ERR_OPERATION_INVALID

[Osier Yang]

于 2011年08月22日 21:54, Eric Blake 写道:

On 08/22/2011 08:12 AM, Osier Yang wrote:

* src/qemu/qemu_monitor_text.c: Error like this function is not
supported by the connection driver is confused obviously.
---
src/qemu/qemu_monitor_text.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)


ACK.

You are correct that VIR_ERR_NO_SUPPORT should only be used in 
libvirt.c, and not in any of the drivers.



Pushed

Osier

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

[libvirt] problem of escaped scancodes

[Lai Jiangshan]

Hi, Daniel P. Berrange,

Our user found that some keycode can not be handled well when
they try to map other keycode to xt_kbd keycode, when xt_kbd keycode
is an escaped scancode.

The xt_kbd keycode http://git.gnome.org/browse/gtk-vnc/plain/src/keymaps.csv
are come from x86_keycodes[] of linux/drivers/char/keyboard.c.
And x86_keycodes[] are:

static const unsigned short x86_keycodes[256] =
{ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79,
 80, 81, 82, 83, 84,118, 86, 87, 88,115,120,119,121,112,123, 92,
284,285,309,  0,312, 91,327,328,329,331,333,335,336,337,338,339,
367,288,302,304,350, 89,334,326,267,126,268,269,125,347,348,349,
360,261,262,263,268,376,100,101,321,316,373,286,289,102,351,355,
103,104,105,275,287,279,258,106,274,107,294,364,358,363,362,361,
291,108,381,281,290,272,292,305,280, 99,112,257,306,359,113,114,
264,117,271,374,379,265,266, 93, 94, 95, 85,259,375,260, 90,116,
377,109,111,277,278,282,283,295,296,297,299,300,301,293,303,307,
308,310,313,314,315,317,318,319,320,357,322,323,324,325,276,330,
332,340,365,342,343,344,345,346,356,270,341,368,369,370,371,372 };


There is no keycode in range [128, 256] in this table, which means
all escaped scancode e0 ?? are encoded as 0x100 | ?? in this table.
Example, LeftCtrl is 0x1d, and RightCtrl is escaped: e0 1d,
RightCtrl is encoded as 0x100 | 0x1d(=0x11d=285) in this table.
The code also tell me the same information:

code = x86_keycodes[keycode];
if (!code)
return -1;

if (code  0x100)
put_queue(vc, 0xe0);
put_queue(vc, (code  0x7f) | up_flag);



But some other place, escaped scancode e0 ?? are encoded as 0x80 | ??,
this encoding is more commonly used, and qemu uses this encoding,
RightCtrl is encoded as 0x80 | 0x1d(=0x9d=157) in qemu:

{ 0x9d, ctrl_r },

keycode = keycodes[i];
if (keycode  0x80)
kbd_put_keycode(0xe0);
kbd_put_keycode(keycode  0x7f);


The problem:
keymaps.csv uses the first encoding while qemu uses the second one, it
causes virsh send-key command can't work when it sends escaped scancodes.

Example:
When do virsh send-key domain KEY_RIGHTCTRL, qemu will receive keycode=285
which is not expected.

So I suggest to change keymaps.csv.
Covert the old
KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,285,228,VK_RCONTROL,0xa3,0x65,0x65
to new
KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,157,228,VK_RCONTROL,0xa3,0x65,0x65
(ditto for other escaped scancodes)

Or just add the new line to keymaps.csv followed by the old line, and make
285 and 157 can work at the same time.

Thought?

Thanks,
Lai.

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

[libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

Error code VIR_ERR_NO_SUPPORT will be translated to this function
is not supported by the connection driver, however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.

The modification can be grouped to 3 following groups:

1) The error intends to tell user it's invalid operation.

 s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.

2) The error intends to tell the configuration of domain
   is not supported.

 s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

3) The error intends to tell the function is not implemented
   on some platform.

 * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
 * and add error strings

 e.g.

static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
-lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+lxcError(VIR_ERR_OPERATION_INVALID, %s,
+ _(interface stats not implemented on this platform));
return -1;
}

[PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in
[PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use

Regards
Osier

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

[libvirt] [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

---
 src/storage/storage_backend.c |   12 ++--
 src/storage/storage_backend_disk.c|2 +-
 src/storage/storage_backend_fs.c  |2 +-
 src/storage/storage_backend_logical.c |2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 889f530..72b37a1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 virCheckFlags(0, -1);
 
 if (vol-target.encryption != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   %s, _(storage pool does not support encrypted 

   volumes));
 goto cleanup;
@@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
 conn-secretDriver-lookupByUUID == NULL ||
 conn-secretDriver-defineXML == NULL ||
 conn-secretDriver-setValue == NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT, %s,
+virStorageReportError(VIR_ERR_OPERATION_INVALID, %s,
   _(secret storage not supported));
 goto cleanup;
 }
@@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
 
 if (vol-target.format != VIR_STORAGE_FILE_QCOW 
 vol-target.format != VIR_STORAGE_FILE_QCOW2) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   _(qcow volume encryption unsupported with 
 volume format %s), type);
 return -1;
@@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
 enc = vol-target.encryption;
 if (enc-format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW 
 enc-format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   _(unsupported volume encryption format %d),
   vol-target.encryption-format);
 return -1;
@@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 return -1;
 }
 if (vol-backingStore.path != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT, %s,
+virStorageReportError(VIR_ERR_OPERATION_INVALID, %s,
   _(copy-on-write image not supported with 
   qcow-create));
 return -1;
 }
 if (vol-target.encryption != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   %s, _(encrypted volumes not supported with 
   qcow-create));
 return -1;
diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index 82b41ef..0eb34b9 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 };
 
 if (vol-target.encryption != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   %s, _(storage pool does not support encrypted 

   volumes));
 return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index ff5afaa..4f53d3f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
 
 if (inputvol) {
 if (vol-target.encryption != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   %s, _(storage pool does not support 
   building encrypted volumes from 
   other volumes));
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index ca4166d..a35b360 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
 const char **cmdargv = cmdargvnew;
 
 if (vol-target.encryption != NULL) {
-virStorageReportError(VIR_ERR_NO_SUPPORT,
+virStorageReportError(VIR_ERR_OPERATION_INVALID,
   %s, _(storage pool does not support encrypted 

   volumes));
 return -1;
-- 
1.7.6

--
libvir-list mailing list
libvir-list@redhat.com

[libvirt] [PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

---
 src/remote/remote_driver.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e5bfa4b..28f333b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2928,7 +2928,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
 remoteDriverLock(priv);
 
 if (priv-domainEventState-timer  0) {
- remoteError(VIR_ERR_NO_SUPPORT, %s, _(no event support));
+ remoteError(VIR_ERR_OPERATION_INVALID, %s, _(no event support));
  goto done;
 }
 
@@ -3277,7 +3277,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t 
*value_size,
 
 /* internalFlags intentionally do not go over the wire */
 if (internalFlags) {
-remoteError(VIR_ERR_NO_SUPPORT, %s, _(no internalFlags support));
+remoteError(VIR_ERR_OPERATION_INVALID, %s, _(no internalFlags 
support));
 goto done;
 }
 
@@ -3541,7 +3541,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr 
conn,
 remoteDriverLock(priv);
 
 if (priv-domainEventState-timer  0) {
- remoteError(VIR_ERR_NO_SUPPORT, %s, _(no event support));
+ remoteError(VIR_ERR_OPERATION_INVALID, %s, _(no event support));
  goto done;
 }
 
-- 
1.7.6

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

[libvirt] [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

---
 src/nodeinfo.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index cae2564..b21e555 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, 
virNodeInfoPtr nodeinfo) {
 }
 #else
 /* XXX Solaris will need an impl later if they port QEMU driver */
-nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+nodeReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(node info not implemented on this platform));
 return -1;
 #endif
@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
 return ret;
 }
 #else
-nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+nodeReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(node CPU stats not implemented on this platform));
 return -1;
 #endif
@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
 # if HAVE_NUMACTL
 if (numa_available()  0) {
 # endif
-nodeReportError(VIR_ERR_NO_SUPPORT,
+nodeReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(NUMA not supported on this host));
 return -1;
 # if HAVE_NUMACTL
@@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
 return ret;
 }
 #else
-nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+nodeReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(node memory stats not implemented on this platform));
 return -1;
 #endif
@@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
 int maxCell;
 
 if (numa_available()  0) {
-nodeReportError(VIR_ERR_NO_SUPPORT,
+nodeReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(NUMA not supported on this host));
 goto cleanup;
 }
@@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
 int n;
 
 if (numa_available()  0) {
-nodeReportError(VIR_ERR_NO_SUPPORT,
+nodeReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(NUMA not supported on this host));
 goto cleanup;
 }
@@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn 
ATTRIBUTE_UNUSED,
   int startCell ATTRIBUTE_UNUSED,
   int maxCells ATTRIBUTE_UNUSED)
 {
-nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+nodeReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(NUMA memory information not available on this 
platform));
 return -1;
 }
 
 unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
 {
-nodeReportError(VIR_ERR_NO_SUPPORT, %s,
+nodeReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(NUMA memory information not available on this 
platform));
 return 0;
 }
-- 
1.7.6

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

[libvirt] [PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

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

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b3e24b4..3dfe305 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4082,7 +4082,7 @@ testStorageFindPoolSources(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 break;
 
 default:
-testError(VIR_ERR_NO_SUPPORT,
+testError(VIR_ERR_OPERATION_INVALID,
   _(pool type '%s' does not support source discovery), type);
 }
 
-- 
1.7.6

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

[libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

* src/qemu/qemu_command.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

* src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/

* src/qemu/qemu_process.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
---
 src/qemu/qemu_command.c |4 ++--
 src/qemu/qemu_driver.c  |   16 
 src/qemu/qemu_process.c |4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbfc7d9..287ad8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 switch(console-targetType) {
 case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
 if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
-qemuReportError(VIR_ERR_NO_SUPPORT, %s,
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 _(virtio channel requires QEMU to support -device));
 goto error;
 }
@@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 break;
 
 default:
-qemuReportError(VIR_ERR_NO_SUPPORT,
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 _(unsupported console target type %s),
 
NULLSTR(virDomainChrConsoleTargetTypeToString(console-targetType)));
 goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..fc2538a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned 
int flags) {
 vm = NULL;
 } else {
 #endif
-qemuReportError(VIR_ERR_NO_SUPPORT, %s,
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(Reboot is not supported without the JSON monitor));
 #if HAVE_YAJL
 }
@@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
   cpumap, maplen, maxcpu)  0)
 goto cleanup;
 } else {
-qemuReportError(VIR_ERR_NO_SUPPORT,
+qemuReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(cpu affinity is not supported));
 goto cleanup;
 }
@@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
 goto cleanup;
 }
 } else {
-qemuReportError(VIR_ERR_NO_SUPPORT,
+qemuReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(cpu affinity is not available));
 goto cleanup;
 }
@@ -5637,7 +5637,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
 }
 
 if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-qemuReportError(VIR_ERR_NO_SUPPORT, _(blkio cgroup isn't 
mounted));
+qemuReportError(VIR_ERR_OPERATION_INVALID, _(blkio cgroup isn't 
mounted));
 goto cleanup;
 }
 
@@ -5790,7 +5790,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
 }
 
 if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-qemuReportError(VIR_ERR_NO_SUPPORT, _(blkio cgroup isn't 
mounted));
+qemuReportError(VIR_ERR_OPERATION_INVALID, _(blkio cgroup isn't 
mounted));
 goto cleanup;
 }
 
@@ -6887,8 +6887,8 @@ qemudDomainInterfaceStats (virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats 
ATTRIBUTE_UNUSED)
 {
-qemuReportError(VIR_ERR_NO_SUPPORT,
-%s, __FUNCTION__);
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
+_(interface stats not implemented on this platform));
 return -1;
 }
 #endif
@@ -8004,7 +8004,7 @@ qemuCPUCompare(virConnectPtr conn,
 qemuDriverLock(driver);
 
 if (!driver-caps || !driver-caps-host.cpu) {
-qemuReportError(VIR_ERR_NO_SUPPORT,
+qemuReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(cannot get host CPU capabilities));
 } else {
 ret = cpuCompareXML(driver-caps-host.cpu, xmlDesc);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6f54b30..616c8e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -261,7 +261,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
 if (conn-secretDriver == NULL ||
 conn-secretDriver-lookupByUUID == NULL ||
 conn-secretDriver-getValue == NULL) {
-qemuReportError(VIR_ERR_NO_SUPPORT, %s,
+qemuReportError(VIR_ERR_OPERATION_INVALID, %s,
 _(secret storage not supported));
 goto cleanup;
 }
@@ -1464,7 +1464,7 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
 return 0;
 
 if (priv-vcpupids == NULL) {
-

[libvirt] [PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/

Special case is changes on lxcDomainInterfaceStats, if it's not
implemented on the platform, prints error like:

lxcError(VIR_ERR_OPERATION_INVALID, %s,
 _(interface stats not implemented on this platform));

As the function is supported by driver actually, error like
VIR_ERR_NO_SUPPORT is confused.
---
 src/lxc/lxc_driver.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a596945..5587b06 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -421,7 +421,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, 
const char *xml)
 goto cleanup;
 
 if ((def-nets != NULL)  !(driver-have_netns)) {
-lxcError(VIR_ERR_NO_SUPPORT,
+lxcError(VIR_ERR_OPERATION_INVALID,
  %s, _(System lacks NETNS support));
 goto cleanup;
 }
@@ -728,7 +728,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned 
long newmem) {
 }
 
 if (driver-cgroup == NULL) {
-lxcError(VIR_ERR_NO_SUPPORT,
+lxcError(VIR_ERR_OPERATION_INVALID,
  %s, _(cgroups must be configured on the host));
 goto cleanup;
 }
@@ -1710,7 +1710,7 @@ static int lxcDomainStartWithFlags(virDomainPtr dom, 
unsigned int flags)
 }
 
 if ((vm-def-nets != NULL)  !(driver-have_netns)) {
-lxcError(VIR_ERR_NO_SUPPORT,
+lxcError(VIR_ERR_OPERATION_INVALID,
  %s, _(System lacks NETNS support));
 goto cleanup;
 }
@@ -1786,7 +1786,7 @@ lxcDomainCreateAndStart(virConnectPtr conn,
 goto cleanup;
 
 if ((def-nets != NULL)  !(driver-have_netns)) {
-lxcError(VIR_ERR_NO_SUPPORT,
+lxcError(VIR_ERR_OPERATION_INVALID,
  %s, _(System lacks NETNS support));
 goto cleanup;
 }
@@ -2519,7 +2519,8 @@ static int
 lxcDomainInterfaceStats(virDomainPtr dom,
 const char *path ATTRIBUTE_UNUSED,
 struct _virDomainInterfaceStats *stats 
ATTRIBUTE_UNUSED)
-lxcError(VIR_ERR_NO_SUPPORT, %s, __FUNCTION__);
+lxcError(VIR_ERR_OPERATION_INVALID, %s,
+ _(interface stats not implemented on this platform));
 return -1;
 }
 #endif
-- 
1.7.6

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

[libvirt] [PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in node_device_conf

[Osier Yang]

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

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 548bbff..4803e2a 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1277,7 +1277,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
 }
 
 if (cap == NULL) {
-virNodeDeviceReportError(VIR_ERR_NO_SUPPORT,
+virNodeDeviceReportError(VIR_ERR_OPERATION_INVALID,
  %s, _(Device is not a fibre channel HBA));
 ret = -1;
 } else if (*wwnn == NULL || *wwpn == NULL) {
-- 
1.7.6

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

[libvirt] [PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

---
 src/xen/xen_hypervisor.c |4 ++--
 src/xen/xend_internal.c  |   26 +-
 src/xen/xm_internal.c|3 ++-
 src/xenxs/xen_sxpr.c |4 ++--
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index cb22b89..77085c9 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
 xenUnifiedUnlock(priv);
 return ret;
 #else
-virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
 block statistics not supported on this platform,
 dom-id);
 return -1;
@@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
 
 return linuxDomainInterfaceStats(path, stats);
 #else
-virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
 /proc/net/dev: Interface not found, 0);
 return -1;
 #endif
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 6d5a854..f44d674 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2793,14 +2793,14 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const 
char *xml,
 goto cleanup;
 }
 } else {
-virXendError(VIR_ERR_NO_SUPPORT, %s,
+virXendError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(unsupported device type));
 goto cleanup;
 }
 break;
 
 default:
-virXendError(VIR_ERR_NO_SUPPORT, %s,
+virXendError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(unsupported device type));
 goto cleanup;
 }
@@ -2921,7 +2921,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const 
char *xml,
 break;
 
 default:
-virXendError(VIR_ERR_NO_SUPPORT, %s,
+virXendError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(unsupported device type));
 goto cleanup;
 }
@@ -3032,7 +3032,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const 
char *xml,
 if (xenFormatSxprOnePCI(dev-data.hostdev, buf, 1)  0)
 goto cleanup;
 } else {
-virXendError(VIR_ERR_NO_SUPPORT, %s,
+virXendError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(unsupported device type));
 goto cleanup;
 }
@@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
 
 /* Xen doesn't support renaming domains during migration. */
 if (dname) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(xenDaemonDomainMigrate: Xen does not support
  renaming domains during migration));
 return -1;
@@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
  * ignores it.
  */
 if (bandwidth) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(xenDaemonDomainMigrate: Xen does not support
  bandwidth limits during migration));
 return -1;
@@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
  * a nice error message.
  */
 if (flags  VIR_MIGRATE_PAUSED) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(xenDaemonDomainMigrate: xend cannot migrate 
paused domains));
 return -1;
 }
@@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
 /* XXX we could easily do tunnelled  peer2peer migration too
if we want to. support these... */
 if (flags != 0) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(xenDaemonDomainMigrate: unsupported flag));
 return -1;
 }
@@ -3569,7 +3569,7 @@ xenDaemonGetSchedulerType(virDomainPtr domain, int 
*nparams)
 /* Support only xendConfigVersion =4 */
 priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
 if (priv-xendConfigVersion  4) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(unsupported in xendConfigVersion  4));
 return NULL;
 }
@@ -3645,7 +3645,7 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
 /* Support only xendConfigVersion =4 */
 priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
 if (priv-xendConfigVersion  4) {
-virXendError(VIR_ERR_NO_SUPPORT,
+virXendError(VIR_ERR_OPERATION_INVALID,
   %s, _(unsupported in xendConfigVersion  4));
 return (-1);
 }
@@ -3752,7 +3752,7 @@ xenDaemonSetSchedulerParameters(virDomainPtr domain,
 /* Support only xendConfigVersion =4 and active 

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Stefan Hajnoczi]

On Wed, Aug 10, 2011 at 11:08 PM, Eric Blake ebl...@redhat.com wrote:
 disk snapshot: the state of a virtual disk used at a given time; once a
 snapshot exists, then it is possible to track a delta of changes that have
 happened since that time.

Did you go into details of the delta API anywhere?  I don't see it.

My general feedback is that you are trying to map all supported
semantics which becomes very complex.  However, I'm a little concerned
that this API will require users to become experts in
snapshots/checkpoints.  You've mentioned quite a few exceptions where
a force flag is needed or other action is required.  Does it make
sense to cut this down to a common abstraction that mortals can use?
:)

Regarding LVM, btrfs, etc support: eventually it would be nice to
support these storage systems as well as storage appliances (various
SAN and NAS boxes that have their own APIs).  If you lay down an
interface that must be implemented in order to enable snapshots on a
given storage system, then others can contribute the actual drivers
for storage systems they care about.

Stefan

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

[libvirt] [test-API][PATCH] Fix the missing ret variable problem

[Wayne Sun]

---
 repos/remoteAccess/tls_setup.py |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/repos/remoteAccess/tls_setup.py b/repos/remoteAccess/tls_setup.py
index 80d6b42..4e7f24e 100644
--- a/repos/remoteAccess/tls_setup.py
+++ b/repos/remoteAccess/tls_setup.py
@@ -343,6 +343,7 @@ def request_credentials(credentials, user_data):
 def hypervisor_connecting_test(uri, auth_tls, username,
 password, logger, expected_result):
  connect remote server 
+ret = 0
 try:
 conn = connectAPI.ConnectAPI()
 if auth_tls == 'none':
@@ -355,6 +356,7 @@ def hypervisor_connecting_test(uri, auth_tls, username,
 except LibvirtAPI, e:
 logger.error(API error message: %s, error code is %s % \
  (e.response()['message'], e.response()['code']))
+ret = 1
 
 conn.close()
 
-- 
1.7.1

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

Re: [libvirt] RFC: managing pci passthrough usage of sriov VFs via a new network forward type

[Gerhard Stenzel]

On Mon, 2011-08-22 at 05:17 -0400, Laine Stump wrote:
 For some reason beyond my comprehension, the designers of SRIOV ethernet 
 cards decided that the virtual functions (VF) of the card (each VF 
 corresponds to an ethernet device, e.g. eth10) should each be given a 
 new+different+random MAC address each time the hardware is rebooted. 

I read this is to avoid wasting MAC addresses from the vendor's pool
which might never be used

 Normally, udev keeps a persistent table that associates each known MAC 
 address with an ethernet device name - any time an ethernet device with 
 a previously-unknown MAC address is found, a new device name is 
 allocated (eth11, etc) and the newly found MAC address is associated 
 with that device name. When an ethernet device is an SRIOV VF, though, 
 udev doesn't persist the MAC address, so at each boot a device is found 
 with a new MAC addres, but the device name from the previous boot is 
 unused so magically the device ends up with the same name even though 
 the MAC address has changed.

RHEL 6.1 seems to use the PCI id to manage the inteface name
in /etc/udev/rules.d/70-persistent-net.rules:

# PCI device 0x8086:0x10ed (ixgbevf)
SUBSYSTEM==net, ACTION==add, ATTR{dev_id}==0x0,
KERNELS==:15:10.0, ATTR{type}==1, KERNEL==eth*, NAME=eth8

 When this device is assigned to a guest via PCI passthrough, though, the 
 guest doesn't have the necessary information to realize that it's 
 actually an SRIOV VF, so the guest's udev persists the MAC address - on 
 the first boot of host+guest, the guest will see it has, e.g., mac 
 address 11:22:33:44:55:66 and udev will add an entry to its persistent 
 table remembering that 11:22:33:44:55:66=eth0. If the host reboots, 
 though, the VF will get a new MAC address, and when the guest boots, it 
 will see a new MAC address (e.g. 66:55:44:33:22:11) and think that 
 there's a different card, so it will create a new device (and a new udev 
 entry - 66:55:44:33:22:11=eth1). This will repeat each time the host 
 reboots, with the obvious undesired consequences.
 
 This makes using SRIOV VFs via PCI passthrough very unpalatable. The 
 problem can be solved by setting the MAC address of the ethernet device 
 prior to assigning it to the guest, but of course the hostdev element 
 used to assign PCI devices to guests has no place to specify a MAC 
 address (and I'm not sure it would be appropriate to add something that 
 function-specific to hostdev). Dave Allan and I have discussed a 
 different possible method of eliminating this problem (using a new 
 forward type for libvirt networks) that I've outlined below. Please let 
 me know what you think - is this reasonable in general? If so, what 
 about the details? If not, any counter-proposals to solve the problem?
 
 Providing Predictable/Configurable MAC Addresses for SRIOV VFs used via 
 PCI Passthrough:
 
 1) network will have a new forward type='hardware'. When forward 
 type='hardware', a pool of ethernet interfaces can be specified, just as 
 for the forward types bridge, vepa, private, and passthrough. At 
 this point, that's the only thing that I've determined is needed in the 
 network definition.

type='hostdev'?

 
 2) In a domain's interface definition, when type='network', if the 
 network has a forward type='hardware', the domain code will request an 
 unused ethernet device from the network driver, then do the following:
 
 3) save the ethernet device name in interface/actual so that it can be 
 easily retrieved if libvirtd is restarted
 
 4) Set the MAC address of the given ethernet device according to the 
 domain interface config.
 
 5) Use the NodeDevice API to learn all the necessary PCI 
 domain/slot/bus/function and add a (non-persisting) hostdev element to 
 the guest's config before starting it up.
 
 6) When the guest is eventually destroyed, the ethernet device will be 
 free'd back to the network pool for use by another guest.
 
 One problem this doesn't solve is that when a guest is migrated, the PCI 
 info for the allocated ethernet device on the destination host will 
 almost surely be different. Is there any provision for dealing with this 
 in the device passthrough code? If not, then migration will still not be 
 possible.
 
 Although I realize that many people are predisposed to not like the idea 
 of PCI passthrough of ethernet devices (including me), it seems that 
 it's going to be used, so we may as well provide the management tools to 
 do it in a sane manner.

If I understand this correctly, this outlines an implicit pci
passthrough and there is no need to provide an explicit hostdev/
element in the domain xml. Guest configs using an explicit hostdev/
element would still expose the problem outlined above, correct?
Any plans for those?

 
 --
 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] RFC: managing pci passthrough usage of sriov VFs via a new network forward type

[Daniel P. Berrange]

On Mon, Aug 22, 2011 at 05:17:25AM -0400, Laine Stump wrote:
 For some reason beyond my comprehension, the designers of SRIOV
 ethernet cards decided that the virtual functions (VF) of the card
 (each VF corresponds to an ethernet device, e.g. eth10) should
 each be given a new+different+random MAC address each time the
 hardware is rebooted. 

[...snip...]

 This makes using SRIOV VFs via PCI passthrough very unpalatable. The
 problem can be solved by setting the MAC address of the ethernet
 device prior to assigning it to the guest, but of course the
 hostdev element used to assign PCI devices to guests has no place
 to specify a MAC address (and I'm not sure it would be appropriate
 to add something that function-specific to hostdev).

In discussions at the KVM forum, other related problems were
noted too. Specifically when using an SRIOV VF with VEPA/VNLink
we need to be able to set the port profile on the VF before
assigning it to the guest, to lock down what the guest can
do. We also likely need to a specify a VLAN tag on the NIC.
The VLAN tag is actally something we need to be able todo
for normal non-PCI passthrough usage of SRIOV networks too.

 Dave Allan
 and I have discussed a different possible method of eliminating this
 problem (using a new forward type for libvirt networks) that I've
 outlined below. Please let me know what you think - is this
 reasonable in general? If so, what about the details? If not, any
 counter-proposals to solve the problem?

The issue I see is that if an application wants to know what
PCI devices have been assigned to a guest, they can no longer
just look at hostdev elements. They also need to look at
interface elements. If we follow this proposed model in other
areas, we could end up with PCI devices appearing as disks
controllers and who knows what else. I think this is not
very desirable for applications, and it is also not good for
our internal code that manages PCI devices. ie the security
drivers now have to look at many different places to find
what PCI devices need labelling.

 One problem this doesn't solve is that when a guest is migrated, the
 PCI info for the allocated ethernet device on the destination host
 will almost surely be different. Is there any provision for dealing
 with this in the device passthrough code? If not, then migration
 will still not be possible.

Migration is irrelevant with PCI passthrough, since we reject any
attempt to migrate a guest with assigned PCI devices. A management
app must explicitly hot-unplug all PCI devices before doing any
migration, and plug back in new ones after migration finishes.

 Although I realize that many people are predisposed to not like the
 idea of PCI passthrough of ethernet devices (including me), it seems
 that it's going to be used, so we may as well provide the management
 tools to do it in a sane manner.

Reluctantly I think we need to provide the neccessary information
underneath the hostdev element. Fortunately we already have an
XML schema for port profile and such things, that we share between
the interface device element and the network schema. 

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

[libvirt] Notes from the KVM Forum relevant to libvirt

[Daniel P. Berrange]

I was at the KVM Forum / LinuxCon last week and there were many
interesting things discussed which are relevant to ongoing libvirt
development. Here was the list that caught my attention. If I have
missed any, fill in the gaps

 - Sandbox/container KVM.  The Solaris port of KVM puts QEMU inside
   a zone so that an exploit of QEMU can't escape into the full OS.
   Containers are Linux's parallel of Zones, and while not nearly as
   secure yet, it would still be worth using more containers support
   to confine QEMU.

 - Events for object changes. We already have async events for virDomainPtr.
   We need the same for virInterfacePtr, virStoragePoolPtr, virStorageVolPtr
   and virNodeDevPtr, so that at the very least applications can be notified
   when objects are created or removed. For virNodeDevPtr we also want to
   be notified when properties change (ie CDROM media change).

 - CGroups passthrough. There is alot of experimentation with cgroups. We
   don't want to expose cgroups as a direct concept in the libvirt API,
   but we should consider putting a generic cgroups get/set in the
   libvirt-qemu.so library, or create a libvirt-linux.so library.
   Also likely add a linux:cgroups XML element to store arbitrary
   tunables in the XML. Same (low) level of support as with qemu:XXX
   of course

 - CPUSet for changing CPU + Memory NUMA pinning. The CPUset cgroups
   controller is able to actually move a guest's memory between NUMA
   nodes. We can already change VCPU pinning, but we need a new API
   to do node pinning of the whole VM, so we can ensure the I/O threads
   are also moved. We also need an API to move the memory pinning to
   new nodes.

 - Guest NUMA topology. If we have guests with RAM size  node size,
   we need to expose a NUMA topology into the guest. The CPU/memory
   pinning APIs will also need to be able to pin individual guest
   NUMA nodes to individual host NUMA nodes.

 - AHCI controller. IDE is going the way of the dodo. We need to add
   support for QEMU's new AHCI controller. This is quite simple, we
   already have a 'sata' disk type we can wire up to QEMU

 - VFIO PCI passthru. The current PCI assignment code may well be
   changed to use something called 'VFIO'. This will need some
   work in libvirt to support new CLI arg syntax, and probably
   some SELinux work

 - QCow3. There will soon be a QCow3 format. We need to add code to
   detect it and extract backing stores, etc. Trivial since the primary
   header format will still be the same as QCow2.

 - QMP completion. Given anthony's plan for a complete replacement of
   the current CLI + monitor syntax in QEMU 2.0 (long way out), he has
   dropped objections to adding new commands to QMP in the near future.
   So all existing HMP commands will immediately be made available in
   QMP with no attempt to re-design them now. So the need for the HMP
   passthrough command will soon go away.

 - Migration + VEPA/VNLink failures. As raised previously on this list,
   Cisco really wants libvirt to have the ability to do migration, and
   optionally *not* fail, even if the VEPA/VNLink setup fails. This will
   require an event notification to the app if a failure of a device
   backend occurs, and an API to let the admin app fix the device backend
   (virDomainUpdateDevice) and some way to tell migration what bits are
   allowed to fail.

 - Virtio SCSI. We need to support this new stuff in QEMU when it is
   eventually implemented. It will mean we avoid the PCI slot usage
   problems inherant in virtio-blk, and get other things like multipath
   and decent SCSI passthrough support.

 - USB 2.0. We need to support this in libvirt asap. It is very important
   for desktop experiance and to support better integration with SPICE
   This also gets us proper USB port addressing. Fun footnote, QEMU USB
   has *never* supported migration. The USB tablet only works by sheer
   luck, as OS' see the device disappear on migration  come back with
   different device ID/port addr and so does a re-initialize !

 - Native KVM tool. The problem statement was that the QEMU code is too
   big/complex  and command line args are too complex, so lets rewrite
   from scratch to make the code small  CLI simple. They achieve this,
   but of course primarily because they lack so many features compared
   to QEMU. They had libvirt support as a bullet point on their preso,
   but I'm not expecting it to replace the current QEMU KVM support in
   the forseeable future, given its current level of features and the
   size of its dev team compared to QEMU/KVM. They did have some fun
   demos of booting using the host OS filesystem though. We can
   actually do the same with regular KVM/libvirt but there's no nice
   demo tool to show it off. I'm hoping to create one

 - Shared memory devices. Some people doing high performance work are
   using the QEMU shared memory device. We don't support this (ivhshm
   device) in libvirt yet. Fairly niche use 

Re: [libvirt] RFC: managing pci passthrough usage of sriov VFs via a new network forward type

[D. Herrendoerfer]

On Aug 23, 2011, at 12:50 PM, Daniel P. Berrange wrote:


[...snip...]


This makes using SRIOV VFs via PCI passthrough very unpalatable. The
problem can be solved by setting the MAC address of the ethernet
device prior to assigning it to the guest, but of course the
hostdev element used to assign PCI devices to guests has no place
to specify a MAC address (and I'm not sure it would be appropriate
to add something that function-specific to hostdev).


In discussions at the KVM forum, other related problems were
noted too. Specifically when using an SRIOV VF with VEPA/VNLink
we need to be able to set the port profile on the VF before
assigning it to the guest, to lock down what the guest can
do. We also likely need to a specify a VLAN tag on the NIC.
The VLAN tag is actally something we need to be able todo
for normal non-PCI passthrough usage of SRIOV networks too.



I guess there is a issue with PCI-passtrough here, If the VEPA link
is set up prior to VM start then that information is lost when the VM
OS resets the device during initialization.
Only on NICs with an integrated bridge can this setup be persistent
because the bridge can handle the VLAN tagging and port setup.
I see a major drawback with storing MAC adresses in hostdev
elements: It would require great care to make sure that MAC adresses
are unique across a big datacenter.


   Dave Allan
and I have discussed a different possible method of eliminating this
problem (using a new forward type for libvirt networks) that I've
outlined below. Please let me know what you think - is this
reasonable in general? If so, what about the details? If not, any
counter-proposals to solve the problem?


The issue I see is that if an application wants to know what
PCI devices have been assigned to a guest, they can no longer
just look at hostdev elements. They also need to look at
interface elements. If we follow this proposed model in other
areas, we could end up with PCI devices appearing as disks
controllers and who knows what else. I think this is not
very desirable for applications, and it is also not good for
our internal code that manages PCI devices. ie the security
drivers now have to look at many different places to find
what PCI devices need labelling.


The same is true for network setups, the available options
are becomming more and more confusing.

Regards,

D.Herrendoerfer

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

Re: [libvirt] Draft document about source control for type of pools

[Daniel P. Berrange]

On Wed, Aug 17, 2011 at 12:53:00PM +0800, Lei Li wrote:
 Hi Daniel,
 
 The last version of my patch fixed bug #611823 prohibit pools with duplicate 
 storage,
 you gave some comments that should do check by source info instead of target 
 path I used.
 These days I view the code and existing documents about storage pool, and 
 tried to
 code based on your comments, but looks like it is turning out to be more 
 complex.
 
 First, I think it is possible to make clear what it means for a pool to be a 
 duplicate.
 Below is the detailed source info be used for each type of pool according to 
 the existing
 documents and the main item to control.
 
 For details of each type of pool:
 
 VIR_STORAGE_POOL_DIR,  /* For dir type, use source-dir only(But in the 
 code and
   example XML file,it use target path).*/
 A pool with a type of dir provides the means to manage files within a 
 directory.

For the type=dir,  the source-dir and target-path are always identical,
so they can be used interchangably in the code. The XML should generally
specify the source dir, and the target path will be automatically filled
in from this.

 VIR_STORAGE_POOL_FS,   /* For fs type, use source-device-path.*/
 This is a variant of the directory pool. Instead of creating a directory on an
 existing mounted filesystem though, it expects a source block device to be 
 named.
 This block device will be mounted and files managed in the directory of its 
 mount point.
 
 VIR_STORAGE_POOL_NETFS,/* For netfs, use host name and source-dir */
 This is a variant of the filesystem pool. Instead of requiring a local block 
 device as
 the source, it requires the name of a host and path of an exported directory. 
 It will
 mount this network filesystem and manage files within the directory of its 
 mount point.
 
 VIR_STORAGE_POOL_LOGICAL,  /* For logical, use source-device-path. */
 This provides a pool based on an LVM volume group. For a pre-defined LVM 
 volume group,
 simply providing the group name is sufficient,while to build a new group 
 requires
 providing a list of source devices to serve as physical volumes.
 
 VIR_STORAGE_POOL_DISK, /* For disk type, use source-device-path.*/
 This provides a pool based on a physical disk. Volumes are created by adding
 partitions to the disk.

These are all correct.

 VIR_STORAGE_POOL_ISCSI,/* For ISCSI type, use source-device-path. */
 This provides a pool based on an iSCSI target.

As well as the 'device' which maps to the iSCSI target name, we also
use the hostname. There is finally also an optional initiator IQN.

If no IQN is specified, then we use the host's default. Uniqueness
checks need to be based on  (hostname, device, iqn)

 VIR_STORAGE_POOL_SCSI, /* For SCSI type, use source-adapter name. */
 This provides a pool based on a SCSI HBA.
 
 VIR_STORAGE_POOL_MPATH,
 This provides a pool that contains all the multipath devices on the host.
 Volume creating is not supported via the libvirt APIs, so we will just
 ignore this type.

These are all correct.

 
 After a preliminary investigation, for source element,
 
source-device-path,
-Pool type: fs, logical, disk, ISCSI.
-May only occur once.
source-dir,
-Pool type: dir, netfs.
-May only occur once.
source-adapter
-Pool type: SCSI.
-May only occur once.
 are the main required location info, and we can control the duplicate storage
 based on these three item above for related type of pool.
 
 Second, I have tried to do check by source info for a test, when I debug, I 
 found
 that pools-objs[]-def-source.* which come from virConnectPtr where all 
 existing
 pools info be keeped == NULL, but the current target pool 
 virStoragePoolDefPtr def
 can get source  field value, so I think libvirt didn't get source field info 
 in the
 pools list where we  do check at all today.
 
 Do you think we should do check based on this? Or any comments for question 
 one and
 two. After you confirmed then I will work on code. Thank you.

Rather than trying to group things according to the source elements
used, group according to the pool type. For each pool type, check the
source information against source info for other pools of the same
type. eg

   if (type == dir) {
  ...check source dir against other pools with type=dir...
   } else if (type == netfs) {
  ...check (host, dir) against other pools with type=netfs...
   } else if (...) {
  
   } else if (type == scsi) {
  ...check adapter against other pools with type=scsi
   }

Finally, there is one extra check which can be applied to all the
file based pools at the same time:

   if (type == dir || type == netfs || type == fs) {
  ..check target path is unique against other pools with
type==dir||netfs||fs...
   }

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- 

Re: [libvirt] RFC: managing pci passthrough usage of sriov VFs via a new network forward type

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 01:53:42PM +0200, D. Herrendoerfer wrote:
 
 On Aug 23, 2011, at 12:50 PM, Daniel P. Berrange wrote:
 
 [...snip...]
 
 This makes using SRIOV VFs via PCI passthrough very unpalatable. The
 problem can be solved by setting the MAC address of the ethernet
 device prior to assigning it to the guest, but of course the
 hostdev element used to assign PCI devices to guests has no place
 to specify a MAC address (and I'm not sure it would be appropriate
 to add something that function-specific to hostdev).
 
 In discussions at the KVM forum, other related problems were
 noted too. Specifically when using an SRIOV VF with VEPA/VNLink
 we need to be able to set the port profile on the VF before
 assigning it to the guest, to lock down what the guest can
 do. We also likely need to a specify a VLAN tag on the NIC.
 The VLAN tag is actally something we need to be able todo
 for normal non-PCI passthrough usage of SRIOV networks too.
 
 
 I guess there is a issue with PCI-passtrough here, If the VEPA link
 is set up prior to VM start then that information is lost when the VM
 OS resets the device during initialization.

IIUC, this is not a problem. When libvirt sets the VEPA/VNLink
information, it does so against the PF of the NIC. When a VF
is reset, it pulls its configuration from a config space in the
PF. So if the guest resets the VF, it'll just reinitialize itself
with the data libvirt set on the PF.

 Only on NICs with an integrated bridge can this setup be persistent
 because the bridge can handle the VLAN tagging and port setup.
 I see a major drawback with storing MAC adresses in hostdev
 elements: It would require great care to make sure that MAC adresses
 are unique across a big datacenter.

Most large scale deployments are going to be using some kind of
management tool that tracks guests across all hosts. Such a tool
has a global view of the network, so can hand out unique MACs
for guests as required.

Libvirt also recently gained support for integrating with lock
managers. We use this to ensure unique access to disk images
currently. We can in theory extend this to do uniqueness checks
on any type of resource associated with a guest. So we could
add locks based on the MAC addresses to avoid duplication.

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/3] qemu: Introduce job queue size limit

[Daniel P. Berrange]

On Tue, Aug 16, 2011 at 06:39:11PM +0200, Michal Privoznik wrote:
 This patch creates an optional BeginJob queue size limit. When
 active, all other attempts above level will fail. To set this
 feature assign desired value to max_queued variable in qemu.conf.
 Setting it to 0 turns it off.
 ---
  src/qemu/libvirtd_qemu.aug |1 +
  src/qemu/qemu.conf |7 +++
  src/qemu/qemu_conf.c   |4 
  src/qemu/qemu_conf.h   |2 ++
  src/qemu/qemu_domain.c |   17 +
  src/qemu/qemu_domain.h |2 ++
  6 files changed, 33 insertions(+), 0 deletions(-)

ACK

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/3] daemon: Create priority workers pool

[Daniel P. Berrange]

On Tue, Aug 16, 2011 at 06:39:10PM +0200, Michal Privoznik wrote:
 This patch annotates APIs with low or high priority.
 In low set MUST be all APIs which might eventually access monitor
 (and thus block indefinitely). Other APIs may be marked as high
 priority. However, some must be (e.g. domainDestroy).
 
 For high priority calls (HPC), there is new thread pool created.
 HPC tries to land in usual thread pool firstly. The condition here
 is it contains at least one free worker. As soon as it doesn't,
 HPCs are placed into the new pool. Therefore, only those APIs which
 are guaranteed to end in reasonable small amount of time can be marked
 as HPC.
 
 The size of this HPC pool is static, because HPC are expected to end
 quickly, therefore jobs assigned to this pool will be served quickly.
 It can be configured in libvirtd.conf via prio_workers variable.
 Default is set to 5.
 
 To mark API with low or high priority, append priority:{low|high} to
 it's comment in src/remote/remote_protocol.x. This is similar to
 autogen|skipgen.



 diff --git a/daemon/remote.c b/daemon/remote.c
 index 939044c..f6c6f35 100644
 --- a/daemon/remote.c
 +++ b/daemon/remote.c
 @@ -464,6 +464,32 @@ int remoteClientInitHook(virNetServerPtr srv 
 ATTRIBUTE_UNUSED,
  return 0;
  }
  
 +int remoteGetProcPriority(virNetMessageHeaderPtr hdr)
 +{
 +u_int prog = hdr-prog;
 +int proc = hdr-proc;
 +int *table = NULL;
 +size_t max = 0;
 +
 +switch (prog) {
 +case REMOTE_PROGRAM:
 +table = remoteProcsPriority;
 +max = remoteNProcs;
 +break;
 +case QEMU_PROGRAM:
 +table = qemuProcsPriority;
 +max = qemuNProcs;
 +break;
 +}
 +
 +if (!table || !max)
 +return 0;
 +if (proc = max)
 +return 0;
 +
 +return table[proc];
 +}


I don't think this is the right way provide the priority information.
We already have a extensible table for information about procedures
in virnetserverprogram.h

struct _virNetServerProgramProc {
virNetServerProgramDispatchFunc func;
size_t arg_len;
xdrproc_t arg_filter;
size_t ret_len;
xdrproc_t ret_filter;
bool needAuth;
};


We should be adding 'unsigned int priority' to the struct.


 diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
 index 0d344e8..a8f9fc6 100755
 --- a/src/rpc/gendispatch.pl
 +++ b/src/rpc/gendispatch.pl
 @@ -954,6 +970,28 @@ elsif ($opt_b) {
  }
  print };\n;
  print size_t ${structprefix}NProcs = 
 ARRAY_CARDINALITY(${structprefix}Procs);\n;
 +
 +# Now we write priority table
 +
 +print \nint ${structprefix}ProcsPriority[] = {\n;
 +for ($id = 0 ; $id = $#calls ; $id++) {
 +if (!defined $calls[$id]) {
 +print 0, /* Unkown proc $id */\n;
 +next;
 +}
 +if ($calls[$id]-{priority}) {
 +print 1;
 +} else {
 +print 0;
 +}
 +if ($calls[$id]-{UC_NAME}) {
 +print , /*  ${procprefix}_PROC_$calls[$id]-{UC_NAME} */;
 +} else {
 +print ,;
 +}
 +print \n;
 +}
 +print };\n;
  }

And so instead of creating a new table here, we should just be writing
the new priority field to the existing table.

  # Bodies for client functions (remote_client_bodies.h).
 diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
 index 1a49dbb..b8150b7 100644
 --- a/src/rpc/virnetserver.c
 +++ b/src/rpc/virnetserver.c
 @@ -72,9 +72,12 @@ struct _virNetServer {
  virMutex lock;
  
  virThreadPoolPtr workers;
 +virThreadPoolPtr prio_workers;
  
  bool privileged;
  
 +virNetServerPriorityProcFunc prio_func;
 +
  size_t nsignals;
  virNetServerSignalPtr *signals;
  int sigread;
 @@ -182,6 +185,7 @@ static int 
 virNetServerDispatchNewMessage(virNetServerClientPtr client,
  {
  virNetServerPtr srv = opaque;
  virNetServerJobPtr job;
 +bool priority = false;
  int ret;
  
  VIR_DEBUG(server=%p client=%p message=%p,
 @@ -192,11 +196,25 @@ static int 
 virNetServerDispatchNewMessage(virNetServerClientPtr client,
  return -1;
  }
  
 +if (srv-prio_func)
 +priority = srv-prio_func(msg-header);
 +
  job-client = client;
  job-msg = msg;
  
  virNetServerLock(srv);
 -if ((ret = virThreadPoolSendJob(srv-workers, job))  0)
 +ret = virThreadPoolSendJob(srv-workers, priority, job);
 +if (ret  -1) {
 +goto cleanup;
 +} else if (ret == -1) {
 +/* try placing job into priority pool */
 +VIR_DEBUG(worker pool full, placing proc %d into priority pool,
 +  msg-header.proc);
 +ret = virThreadPoolSendJob(srv-prio_workers, false, job);
 +}
 +
 +cleanup:
 +if (ret  0)
  VIR_FREE(job);
  virNetServerUnlock(srv);
  

One thing that concerns me is that using 2 separate thread pools is
inherantly racy. eg, between the time we check for free 

Re: [libvirt] ignore vs. error when inappropriate or unrecognized attributes/elements are present in XML

[Daniel P. Berrange]

On Mon, Aug 22, 2011 at 01:52:07PM -0400, Laine Stump wrote:
 On 08/22/2011 09:10 AM, Eric Blake wrote:
 On 08/22/2011 01:36 AM, Laine Stump wrote:
 The problem I was *really* trying to point out is that of a keymap
 attribute being given *at all* when type is something other than spice
 or vnc. The problem is that keymap is stored in a union, and the parts
 of the union that are active when type != (spice|vnc) don't have any
 keymap members. So even if keymap has something that would have been
 valid when type=(spice|vnc) (but not otherwise), it isn't flagged in
 the parser (because the parser ignores keymap when it's not appropriate,
 rather than logging an error) and the driver *can't* do anything about
 it (because it has no way of knowing that a keymap was given - the
 parser can't put keymap into the config object because there's no place
 to put it).
 
 To some degree, rng validation can map unions, if the
 distinguishing choice between unions is an attribute also exposed
 in the xml.  It is done via choice and group, but it does make
 the rng larger; it also helps to have more defines in order to
 avoid some repetition of subelements that are common between more
 than one choice.
 
 
 In particular, to solve
 https://bugzilla.redhat.com/show_bug.cgi?id=638633 according to your
 recommendation, I think what I need to do is:
 
 1) Remove the script attribute from the bridge  ethernet unions in
 virDomainNetDef and place it in the common part of the struct.
 
 2) Change the parser to always populate script, no matter what is the
 type of the virDomainNetDef.
 
 Not necessarily.  We can instead rewrite the rng to forbid script
 attributes from all but bridge and ethernet attributes.  However,
 there's still the issue that some, but not all, hypervisors can
 support scripts with a bridge, so there's still some validation
 that will have to be done in drivers to reject unsupported items
 even though the rng allows it.
 
 
 That's all great, but the fact remains that we don't use the RNG
 anywhere in the libvirt API, so it does us no good, for example,
 when running virsh edit mydomain.
 
 Until/unless we do that (validate input XML to the relevant RNG),
 having RNGs that properly mirror unions in the config objects is
 just preventing us from detecting improper usage. (and I'm doubtful
 this validation is a good candidate for backporting)

As I mentioned earlier in this thread, we can easily add a new
flag to the XML related APIs to request validation against the
RNG. virsh edit would set this flag and if it fails, would
prompt the user whether they want to correct the XML, or force
the change ignoring validation errors.

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] qemu: Allow graceful domain destroy

[Michal Privoznik]

On 22.08.2011 20:31, Daniel P. Berrange wrote:
 On Mon, Aug 22, 2011 at 05:33:12PM +0100, Daniel P. Berrange wrote:
 On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
 On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
 If we had a separate API for sending 'quit' on the monitor, then the
 mgmt app can decide how long to wait for the graceful shutdown of QEMU
 before resorting to the hard virDomainDestroy command. If the app knows
 that there is high I/O load, then it might want to wait for 'quit' to
 complete longer than normal to allow enough time for I/O flush.

 Indeed - that is exactly what I was envisioning with a
 virDomainShutdownFlags() call with a flag to request to use the quit
 monitor command instead of the default ACPI injection.  The
 virDomainShutdownFlags() would have no timeout (it blocks until
 successful, or returns failure with no 'quit' command attempted),
 and the caller can inject their own unconditional virDomainDestroy()
 at whatever timeout they think is appropriate.

 The virDomainShutdown API is really about guest initiated graceful
 shutdown. Sending the 'quit' command to QEMU is still *ungraceful*
 as far as the guest OS is concerned, so I think it is best not to
 leverage the Shutdown API for 'quit'.

 I think this probably calls for a virDomainQuit API.
 
 Actually this entire thread is on the wrong path.
 
 Both the monitor 'quit' command and 'SIGTERM' in QEMU do exactly the
 same thing. A immediate stop of the guest, but with a graceful shutdown
 of the QEMU process[1].
 
 In theory there is a difference that sending a signal is asynchronous
 and 'quit' is a synchronous command, but in practice this is not
 relevant, since while executio nof the 'quit' command is synchronous,
 this command only makes the *request* to exit. QEMU won't actually
 exit until the event loop processes the request later.
 
 There is thus no point in us even bothering with sending 'quit' to
 the QEMU monitor.
 
 The virDomainDestroy method calls qemuProcessKill which sends SIGTERM,
 waits a short while, then sends SIGKILL. It then calls qemuProcessStop
 to reap the process. This also happens to call qemuProcessKill again
 with SIGTERM, then SIGKILL.
 
 We need to make this more controllable by apps, by making it possible
 to send just the SIGTERM and not the SIGKILL. Then we can add a new
 flag to virDomainDestroy to request this SIGTERM only behaviour. If
 the guest does not actually die, the mgmt app can then just reinvoke
 virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL
 behaviour we have today.

Sending signal to qemu process is just a part of domain destroying. What
about cleanup code (emitting event, audit log, removing transient
domain, ...)? Can I rely on monitor EOF handling code?  What should be
the return value for this case when only SIGTERM is sent?
 
 So there's no need for the QEMU monitor to be involved anywhere in
 this.
 
 Regards,
 Daniel
 
 [1] The SIGTERM handler and 'quit' command handler both end up just
 calling qemu_system_shutdown_request().

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

[libvirt] Libvirt 0.9.4rc2 + qemu 0.15 VNC/TLS not working

[Radek Hladik]

Hi all,
I am trying new versions of software for my libvirt/qemu farm. I 
upgraded libvirt to 0.9.3 and I had problems with loading certificates 
for libvirt itself (probably GNUTLS related). Upgrade to 0.9.4rc2 solved 
the problem.
Now I am trying to upgrade qemu (from 0.12.5) to 0.15. I compiled the 
SRPM from rawhide for Fedora 13 (OS on my farms) and I copied the binary 
to one farm, so I can choose qemu version by emulator element in guest 
XML. I had problem with booting off virtio disk, but upgrade of seabios 
to 0.6.2 solved it (I had to make a wrapper for qemu to be able to add 
-bios option though).
But main issue that I am not able to solve is that VNC is not using TLS. 
I've found out these:

* qemu 0.12.5 is working without problems
* there is no error in libvirt or qemu machine log
* If I copy the command from libvirt log, remove the network and monitor 
definition and start it under root, TLS works as expected
* the vnc definition on command line is quite simple:  -vnc 
0.0.0.0:0,password,tls,x509=/etc/pki/libvirt/pki-vnc -k en-us


I am thinking whether there is not a problem with monitor setting 
something after the machine starts. Libvirt does the same with password, 
so maybe it does something with TLS. Unfortunately I do not know whether 
there is any option how to debug the monitor libvirt-qemu communication.


And I have one other question. Is there any way how to interact directly 
with monitor? Qemu has new guest agent that seems to provide some nice 
functions and expose them via monitor and I would like to test it.


Radek

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

Re: [libvirt] [PATCH 3/3] qemu: Deal with stucked qemu on daemon startup

[Daniel P. Berrange]

On Tue, Aug 16, 2011 at 06:39:12PM +0200, Michal Privoznik wrote:
 If libvirt daemon gets restarted and there is (at least) one
 unresponsive qemu, the startup procedure hangs up. This patch creates
 one thread per vm in which we try to reconnect to monitor. Therefore,
 blocking in one thread will not affect other APIs.
 ---
  src/qemu/qemu_driver.c  |   23 +++-
  src/qemu/qemu_process.c |   87 ++
  2 files changed, 85 insertions(+), 25 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 421a98e..4574b6c 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -143,26 +143,15 @@ qemuAutostartDomain(void *payload, const void *name 
 ATTRIBUTE_UNUSED, void *opaq
  
  virDomainObjLock(vm);
  virResetLastError();
 -if (qemuDomainObjBeginJobWithDriver(data-driver, vm,
 -QEMU_JOB_MODIFY)  0) {
 +if (vm-autostart 
 +!virDomainObjIsActive(vm) 
 +qemuDomainObjStart(data-conn, data-driver, vm,
 +   false, false,
 +   data-driver-autoStartBypassCache)  0) {
  err = virGetLastError();
 -VIR_ERROR(_(Failed to start job on VM '%s': %s),
 +VIR_ERROR(_(Failed to autostart VM '%s': %s),
vm-def-name,
err ? err-message : _(unknown error));
 -} else {
 -if (vm-autostart 
 -!virDomainObjIsActive(vm) 
 -qemuDomainObjStart(data-conn, data-driver, vm,
 -   false, false,
 -   data-driver-autoStartBypassCache)  0) {
 -err = virGetLastError();
 -VIR_ERROR(_(Failed to autostart VM '%s': %s),
 -  vm-def-name,
 -  err ? err-message : _(unknown error));
 -}
 -
 -if (qemuDomainObjEndJob(data-driver, vm) == 0)
 -vm = NULL;
  }

I'm not really seeing why this change is safe. You're removing the
job protection from the autostart code, but AFAICT, no where else
in the caller of this method is changed to acquire the job instead.

 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index 21e73a5..1daf6ae 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -820,6 +820,7 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
  {
  qemuDomainObjPrivatePtr priv = vm-privateData;
  int ret = -1;
 +qemuMonitorPtr mon = NULL;
  
  if (virSecurityManagerSetSocketLabel(driver-securityManager, vm)  0) {
  VIR_ERROR(_(Failed to set security context for monitor for %s),
 @@ -831,15 +832,29 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
   * deleted while the monitor is active */
  virDomainObjRef(vm);
  
 -priv-mon = qemuMonitorOpen(vm,
 -priv-monConfig,
 -priv-monJSON,
 -monitorCallbacks);
 +ignore_value(virTimeMs(priv-monStart));
 +virDomainObjUnlock(vm);
 +qemuDriverUnlock(driver);
 +
 +mon = qemuMonitorOpen(vm,
 +  priv-monConfig,
 +  priv-monJSON,
 +  monitorCallbacks);
 +
 +qemuDriverLock(driver);
 +virDomainObjLock(vm);
 +priv-monStart = 0;

I'm also not convinced it is safe to release the driver/domain
locks in this way. At the very *least* you need to be acquiring
an extra reference on the virDomainObjPtr, ideally using the
EnterMonitor/ExitMonitor methods, otherwise some other thread
may destroy the virDomainObjPtr while it is unlocked.

Also what is the 'monStart' field used for ?

  
  /* Safe to ignore value since ref count was incremented above */
 -if (priv-mon == NULL)
 +if (mon == NULL)
  ignore_value(virDomainObjUnref(vm));
  
 +if (!virDomainObjIsActive(vm)) {
 +qemuMonitorClose(mon);
 +goto error;
 +}
 +priv-mon = mon;
 +
  if (virSecurityManagerClearSocketLabel(driver-securityManager, vm)  0) 
 {
  VIR_ERROR(_(Failed to clear security context for monitor for %s),
vm-def-name);
 @@ -2484,21 +2499,25 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
  struct qemuProcessReconnectData {
  virConnectPtr conn;
  struct qemud_driver *driver;
 +void *payload;
  };
  /*
   * Open an existing VM's monitor, re-detect VCPU threads
   * and re-reserve the security labels in use
   */
  static void
 -qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void 
 *opaque)
 +qemuProcessReconnect(void *opaque)
  {
 -virDomainObjPtr obj = payload;
  struct qemuProcessReconnectData *data = opaque;
  struct qemud_driver *driver = data-driver;
 +virDomainObjPtr obj = data-payload;
  qemuDomainObjPrivatePtr priv;
  virConnectPtr conn = data-conn;
  

Re: [libvirt] [PATCH] qemu: Allow graceful domain destroy

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 02:36:02PM +0200, Michal Privoznik wrote:
 On 22.08.2011 20:31, Daniel P. Berrange wrote:
  On Mon, Aug 22, 2011 at 05:33:12PM +0100, Daniel P. Berrange wrote:
  On Mon, Aug 22, 2011 at 09:29:56AM -0600, Eric Blake wrote:
  On 08/22/2011 09:21 AM, Daniel P. Berrange wrote:
  If we had a separate API for sending 'quit' on the monitor, then the
  mgmt app can decide how long to wait for the graceful shutdown of QEMU
  before resorting to the hard virDomainDestroy command. If the app knows
  that there is high I/O load, then it might want to wait for 'quit' to
  complete longer than normal to allow enough time for I/O flush.
 
  Indeed - that is exactly what I was envisioning with a
  virDomainShutdownFlags() call with a flag to request to use the quit
  monitor command instead of the default ACPI injection.  The
  virDomainShutdownFlags() would have no timeout (it blocks until
  successful, or returns failure with no 'quit' command attempted),
  and the caller can inject their own unconditional virDomainDestroy()
  at whatever timeout they think is appropriate.
 
  The virDomainShutdown API is really about guest initiated graceful
  shutdown. Sending the 'quit' command to QEMU is still *ungraceful*
  as far as the guest OS is concerned, so I think it is best not to
  leverage the Shutdown API for 'quit'.
 
  I think this probably calls for a virDomainQuit API.
  
  Actually this entire thread is on the wrong path.
  
  Both the monitor 'quit' command and 'SIGTERM' in QEMU do exactly the
  same thing. A immediate stop of the guest, but with a graceful shutdown
  of the QEMU process[1].
  
  In theory there is a difference that sending a signal is asynchronous
  and 'quit' is a synchronous command, but in practice this is not
  relevant, since while executio nof the 'quit' command is synchronous,
  this command only makes the *request* to exit. QEMU won't actually
  exit until the event loop processes the request later.
  
  There is thus no point in us even bothering with sending 'quit' to
  the QEMU monitor.
  
  The virDomainDestroy method calls qemuProcessKill which sends SIGTERM,
  waits a short while, then sends SIGKILL. It then calls qemuProcessStop
  to reap the process. This also happens to call qemuProcessKill again
  with SIGTERM, then SIGKILL.
  
  We need to make this more controllable by apps, by making it possible
  to send just the SIGTERM and not the SIGKILL. Then we can add a new
  flag to virDomainDestroy to request this SIGTERM only behaviour. If
  the guest does not actually die, the mgmt app can then just reinvoke
  virDomainDestroy without the flag, to get the full SIGTERM+SIGKILL
  behaviour we have today.
 
 Sending signal to qemu process is just a part of domain destroying. What
 about cleanup code (emitting event, audit log, removing transient
 domain, ...)? Can I rely on monitor EOF handling code?  What should be
 the return value for this case when only SIGTERM is sent?

QEMU will send an event on the monitor when it shuts down cleanly
via 'SIGQUIT' - we already handle that.

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: Do not try to free domain if it is NULL

[Eric Blake]

On 08/22/2011 09:54 PM, Osier Yang wrote:

Without these patch, there will be error like below if domain
is NULL.

error: invalid domain pointer in virDomainFree

Which is useless.
---
  tools/virsh.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)




-out:
-virDomainFree(dom);
+cleanup:
+if (dom) virDomainFree(dom);


Split this into two lines:
if (dom)
virDomainFree(dom);

ACK with that style fix.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Eric Blake]

On 08/23/2011 03:52 AM, Stefan Hajnoczi wrote:

On Wed, Aug 10, 2011 at 11:08 PM, Eric Blakeebl...@redhat.com  wrote:

disk snapshot: the state of a virtual disk used at a given time; once a
snapshot exists, then it is possible to track a delta of changes that have
happened since that time.


Did you go into details of the delta API anywhere?  I don't see it.


It's not available yet, because qemu doesn't provide anything yet.  I 
think that APIs to inspect the actual delta disk contents between the 
current state and a prior snapshot will be similar to block pull, but we 
can't implement anything without support from the underlying tools.




My general feedback is that you are trying to map all supported
semantics which becomes very complex.  However, I'm a little concerned
that this API will require users to become experts in
snapshots/checkpoints.  You've mentioned quite a few exceptions where
a force flag is needed or other action is required.  Does it make
sense to cut this down to a common abstraction that mortals can use?


Hopefully I'm making the error messages specific enough in the cases 
where a revert is rejected but would succeed with a force flag.  And as 
I haven't actually implemented the force flag yet, I may still end up 
tweaking things a bit compared to the original RFC when I actually get 
into coding things.



Regarding LVM, btrfs, etc support: eventually it would be nice to
support these storage systems as well as storage appliances (various
SAN and NAS boxes that have their own APIs).  If you lay down an
interface that must be implemented in order to enable snapshots on a
given storage system, then others can contribute the actual drivers
for storage systems they care about.


Yes, there's still quite a bit of refactoring to be done to move the 
snapshot work out of the qemu driver an into the storage volume driver, 
with enough of an expressive interface that the qemu driver can then 
make several calls to probe the snapshot capabilities of each storage 
volume.  But one step at a time, and the first thing to get working is 
proving that the xml changes are sufficient for qemu to do qcow2 
snapshots, and that the xml remains flexible enough to later extend (it 
isn't locking us into a qcow2-only solution).


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCH] virsh: Do not try to free domain if it is NULL

[Osier Yang]

于 2011年08月23日 21:34, Eric Blake 写道:

Without these patch, there will be error like below if domain
is NULL.

error: invalid domain pointer in virDomainFree

Which is useless. 


Pushed with the style changed.

Osier

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

Re: [libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

于 2011年08月23日 17:39, Osier Yang 写道:

Error code VIR_ERR_NO_SUPPORT will be translated to this function
is not supported by the connection driver, however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.

The modification can be grouped to 3 following groups:

1) The error intends to tell user it's invalid operation.

  s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.

2) The error intends to tell the configuration of domain
is not supported.

  s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

3) The error intends to tell the function is not implemented
on some platform.

  * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
  * and add error strings

  e.g.

static int
lxcDomainInterfaceStats(virDomainPtr dom,
 const char *path ATTRIBUTE_UNUSED,
 struct _virDomainInterfaceStats *stats 
ATTRIBUTE_UNUSED)
 -lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
 +lxcError(VIR_ERR_OPERATION_INVALID, %s,
 + _(interface stats not implemented on this platform));
 return -1;
}

[PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in
[PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use

Regards
Osier

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

Argh, pushed these series carelessly. Will revert it if there is
something wrong.

Osier

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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Eric Blake]

On 08/23/2011 04:28 AM, Stefan Hajnoczi wrote:

I forgot to ask the obvious question:

I am writing a backup program that is using the new snapshot APIs.  A
snapshot has been created, how do I read out the data from the
snapshot?


Here's how to access the data in the snapshot, at least for the first 
round implementation of qcow2 snapshots:


If you created an internal snapshot (virDomainSnapshotCreateXML with no 
flags), then the only way right now to read data out is to shut down any 
qemu process (since qemu-img should not be used on a file in active use 
by qemu), then:

qemu-img convert [options] -s snapshot file backup
to extract the named internal snapshot from 'file' into a new file 'backup'.

If you created an external snapshot (virDomainSnapshotCreateXML with the 
new _DISK_ONLY flag), then the data from the snapshot is the old file 
name.  That is, if you start with '/path/to/old', then create a snapshot 
with a target file of '/path/to/new', then /path/to/old _is_ the 
snapshot, and /path/to/new is a qcow2 file with /path/to/old as its 
backing file.  The snapshot (old file) can safely be accessed even while 
qemu is still running.


As for how to access which blocks have changed in the delta since the 
snapshot, that is not yet exposed in libvirt, due to lack of support in 
qemu and qemu-img.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use

[Eric Blake]

On 08/23/2011 07:45 AM, Osier Yang wrote:

于 2011年08月23日 17:39, Osier Yang 写道:

Error code VIR_ERR_NO_SUPPORT will be translated to this function
is not supported by the connection driver, however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.

The modification can be grouped to 3 following groups:

1) The error intends to tell user it's invalid operation.

s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.

2) The error intends to tell the configuration of domain
is not supported.

s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

3) The error intends to tell the function is not implemented
on some platform.

* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings


ACK series - all relatively simple cleanups, and all make sense.


Argh, pushed these series carelessly. Will revert it if there is
something wrong.


No fears - nothing to revert. :)

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use

[Osier Yang]

于 2011年08月23日 21:52, Eric Blake 写道:

On 08/23/2011 07:45 AM, Osier Yang wrote:

于 2011年08月23日 17:39, Osier Yang 写道:

Error code VIR_ERR_NO_SUPPORT will be translated to this function
is not supported by the connection driver, however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.

The modification can be grouped to 3 following groups:

1) The error intends to tell user it's invalid operation.

s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.

2) The error intends to tell the configuration of domain
is not supported.

s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/

3) The error intends to tell the function is not implemented
on some platform.

* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings


ACK series - all relatively simple cleanups, and all make sense.


Argh, pushed these series carelessly. Will revert it if there is
something wrong.


No fears - nothing to revert. :)


Thanks, :-)

Osier

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

Re: [libvirt] problem of escaped scancodes

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 03:32:51PM +0800, Lai Jiangshan wrote:
 
 Hi, Daniel P. Berrange,
 
 Our user found that some keycode can not be handled well when
 they try to map other keycode to xt_kbd keycode, when xt_kbd keycode
 is an escaped scancode.
 
 The xt_kbd keycode http://git.gnome.org/browse/gtk-vnc/plain/src/keymaps.csv
 are come from x86_keycodes[] of linux/drivers/char/keyboard.c.
 And x86_keycodes[] are:
 
 static const unsigned short x86_keycodes[256] =
   { 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15,
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63,
64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79,
80, 81, 82, 83, 84,118, 86, 87, 88,115,120,119,121,112,123, 92,
   284,285,309,  0,312, 91,327,328,329,331,333,335,336,337,338,339,
   367,288,302,304,350, 89,334,326,267,126,268,269,125,347,348,349,
   360,261,262,263,268,376,100,101,321,316,373,286,289,102,351,355,
   103,104,105,275,287,279,258,106,274,107,294,364,358,363,362,361,
   291,108,381,281,290,272,292,305,280, 99,112,257,306,359,113,114,
   264,117,271,374,379,265,266, 93, 94, 95, 85,259,375,260, 90,116,
   377,109,111,277,278,282,283,295,296,297,299,300,301,293,303,307,
   308,310,313,314,315,317,318,319,320,357,322,323,324,325,276,330,
   332,340,365,342,343,344,345,346,356,270,341,368,369,370,371,372 };
 
 
 There is no keycode in range [128, 256] in this table, which means
 all escaped scancode e0 ?? are encoded as 0x100 | ?? in this table.
 Example, LeftCtrl is 0x1d, and RightCtrl is escaped: e0 1d,
 RightCtrl is encoded as 0x100 | 0x1d(=0x11d=285) in this table.
 The code also tell me the same information:
 
   code = x86_keycodes[keycode];
   if (!code)
   return -1;
 
   if (code  0x100)
   put_queue(vc, 0xe0);
   put_queue(vc, (code  0x7f) | up_flag);
 
 
 
 But some other place, escaped scancode e0 ?? are encoded as 0x80 | ??,
 this encoding is more commonly used, and qemu uses this encoding,
 RightCtrl is encoded as 0x80 | 0x1d(=0x9d=157) in qemu:
 
 { 0x9d, ctrl_r },
 
 keycode = keycodes[i];
 if (keycode  0x80)
 kbd_put_keycode(0xe0);
 kbd_put_keycode(keycode  0x7f);
 
 
 The problem:
 keymaps.csv uses the first encoding while qemu uses the second one, it
 causes virsh send-key command can't work when it sends escaped scancodes.

What this acutally means is that we have 2 different scancode sets,
each with their own different encoding for the extended scancodes.
We are simply using the wrong set when talking to QEMU.

 Example:
 When do virsh send-key domain KEY_RIGHTCTRL, qemu will receive keycode=285
 which is not expected.
 
 So I suggest to change keymaps.csv.
 Covert the old
 KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,285,228,VK_RCONTROL,0xa3,0x65,0x65
 to new
 KEY_RIGHTCTRL,97,RightControl,0x3e,,,88,,157,228,VK_RCONTROL,0xa3,0x65,0x65
 (ditto for other escaped scancodes)
 
 Or just add the new line to keymaps.csv followed by the old line, and make
 285 and 157 can work at the same time.

No, we just need to switch to use the correct scancode set.

The original keymap-gen.pl script in GTK-VNC has code for auto-generating
the RFB scancode set, from the data for the XT KBD scancode set. It is a
straighforward re-encoding for the extended keys:

# RFB keycodes are XT kbd keycodes with a slightly
# different encoding of 0xe0 scan codes. RFB uses
# the high bit of the first byte, instead of the low
# bit of the second byte.
rfbkey = (xtkbdkey  0x100)  1 | (ktkbdkey  0x7f)

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] [RFC] NUMA topology specification

[Daniel P. Berrange]

On Fri, Aug 19, 2011 at 12:05:43PM +0530, Bharata B Rao wrote:
 Hi,
 
 qemu supports specification of NUMA topology on command line using -numa 
 option.
 
 -numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]
 
 I see that there is no way to specify such NUMA topology in libvirt
 XML. Are there plans to add support for NUMA topology specification ?
 Is anybody already working on this ? If not I would like to add this
 support for libvirt.
 
 Currently the topology specification available in libvirt ( topology
 sockets='1' cores='2' threads='1'/) translates to -smp
 sockets=1,cores=2,threads=1 option of qemu. There is not equivalent
 in libvirt that could generate -numa command line option of qemu.
 
 How about something like this ? (OPTION 1)
 
 cpu
 ...
 numa nodeid='node' cpus='cpu[-cpu]' mem='size'
 ...
 /cpu
 
 And we could specify multiple such lines, one for each node.

I'm not sure it really makes sense having the NUMA memory config
inside the cpu configuration, but i like the simplicity of
of this specification.

 -numa and -smp options in qemu do not work all that well since they
 are parsed independent of each other and one could specify a cpu set
 with -numa option that is incompatible with sockets,cores and threads
 specified on -smp option. This should be fixed in qemu, but given that
 such a problem has been observed, should libvirt tie the specification
 of numa and smp (sockets,threads,cores) together so that one is forced
 to specify only valid combinations of nodes and cpus in libvirt ?

No matter what we do, libvirt is going to have todo some kind of
semantic validation on the different info.

 May be something like this: (OPTION 2)
 
 cpu
 ...
 topology sockets='1' cores='2' threads='1' nodeid='0' cpus='0-1' mem='size'
 topology sockets='1' cores='2' threads='1' nodeid='1' cpus='2-3' mem='size'
 ...
 /cpu
 
 This should result in a 2 node system with each node having 1 socket
 with 2 cores.

This has the problem of redundancy of specification of the sockets,
cores  threads, vs the new 'cpus' attribute. eg you can specify
wierd configs like:

  topology sockets='1' cores='2' threads='1' nodeid='0' cpus='0-1' mem='size'
  topology sockets='2' cores='1' threads='1' nodeid='1' cpus='2-3' mem='size'

Or even  bogus configs

  topology sockets='1' cores='2' threads='1' nodeid='0' cpus='0-1' mem='size'
  topology sockets='4' cores='1' threads='1' nodeid='1' cpus='2-3' mem='size'

That all said, given our current XML schema, it is inevitable that we
will have some level of duplication of information.


Some things that are important to consider are how this interacts with
possible CPU / memory hotplug in the future, and how we will be able
to pin guest NUMA nodes, to host NUMA nodes.

For the first point, it might be desirable to create a NUMA topology
which supports upto 8 logical CPUs, but only have 2 physical sockets
actually plugged in at boot time.

Also, I dread to question whether we want to be able to represent a
multi-level NUMA topology, or just assume one level. If we want to
be able to cope with multi-level topology, can we assume the levels
are solely grouping at the socket, or will we have to consider the
possibility of NUMA *inside* a socket.

In other words, are we associating socket numbers with NUMA nodes,
or are we associating logical CPU numbers with NUMA nodes.

This is the difference between configuring something like:

  vpus16/vcpus
  cpu
topology sockets='4' cores='4' threads='1'
  /cpu
  numa
node sockets='0-1' mem='0-1024'/
node sockets='2-3' mem='1024-2048'/
  /numa

vs

  vpus16/vcpus
  cpu
topology sockets='4' cores='4' threads='1'
  /cpu
  numa
node cpus='0-7'  mem='0-1024'/
node cpus='8-15' mem='1024-2048'/
  /numa

vs


  vpus16/vcpus
  cpu
topology sockets='4' cores='4' threads='1'
  /cpu
  numa
node mems='0-1024'/
  node cpus='0-3'/
  node cpus='4-7'/
/node
node mems='1024-2048'/
  node cpus='8-11'/
  node cpus='12-15'/
/node
  /numa

vs

  ...more horrible examples...

NB, QEMU's -numa argument may well not support some of the things I
am talking about here, but we need to consider the real possibilty
that QEMU's -numa arg will be extended, or replaced in the future.

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 34/26] snapshot: add disks to snapshot xml

[Eric Blake]

On 08/19/2011 03:58 PM, Eric Blake wrote:

Adds an optional element todomainsnapshot, which will be used
to give user control over external snapshot filenames on input,
and specify generated filenames on output.

@@ -11044,6 +0,19 @@ virDomainSnapshotDefParseString(const char *xmlStr,

  def-description = virXPathString(string(./description), ctxt);

+if ((i = virXPathNodeSet(./disks/*), ctxt,nodes))  0)


Typo.  Squash this in:

diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index c9daebb..40ba564 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -11137,7 +11137,7 @@ virDomainSnapshotDefParseString(const char *xmlStr,

 def-description = virXPathString(string(./description), ctxt);

-if ((i = virXPathNodeSet(./disks/*), ctxt, nodes))  0)
+if ((i = virXPathNodeSet(./disks/*, ctxt, nodes))  0)
 goto cleanup;
 def-ndisks = i;
 if (def-ndisks  VIR_ALLOC_N(def-disks, def-ndisks)  0) {

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Corey Bryant]

On 08/22/2011 03:25 PM, Anthony Liguori wrote:

On 08/22/2011 01:22 PM, Daniel P. Berrange wrote:

On Mon, Aug 22, 2011 at 12:25:25PM -0500, Anthony Liguori wrote:

On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:

On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:

I don't think it makes sense to have qemu-fe do dynamic labelling.
You certainly could avoid the fd passing by having qemu-fe do the
open though and just let qemu-fe run without the restricted security
context.


qemu-fe would also not be entirely simple,


Indeed.


because it will need to act
as a proxy for the monitor, in order to make hotplug work. ie the mgmt
app would be sending 'drive_add file:/foo/bar' to qemu-fe, which would
then have to open the file and send 'drive_add fd:NN' onto the real
QEMU,
and then pass the results on back.

In addition qemu-fe would still have to be under some kind of
restricted
security context for it to be acceptable. This is going to want to
be as
locked down as possible.


I think there's got to be some give and take here.

It should at least be as locked down as libvirtd. From a security
point of view, we should be able to agree that we want libvirtd to
be as locked down as possible.

But there shouldn't be a hard requirement to lock down qemu-fe more
than libvirtd. Instead, the requirement should be for qemu-fe to be
as/more vigilant in not trusting qemu-system-x86_64 as libvirtd is.

The fundamental problem here, is that there is some logic in
libvirtd that rightly belongs in QEMU. In order to preserve the
security model, that means that we're going to have to take a
subsection of QEMU and trust it more.


Well we have a process that makes security decisions, and a process
which applies those security decisions and a process which is confined
by those decisions. Currently libvirtd makes applies the decisions,
and qemu is confined. A qemu-fe model would mean that libvirt is making
the decisions, but is then relying on qemu-fe to apply them. IMHO that
split is undesirable, but that's besides the point, since this is not
a decision that needs to be made now.

'qemu-fe' needs to have a way to communicate with the confined process
('qemu-system-XXX') to supply it the resources (file FDs) it needs to
access. The requirements of such a comms channel for qemu-fe are going
to be the same as those needed by libvirtd talking to QEMU today, or
indeed by any process that is applying security decisions to QEMU.


But the fundamental difference is that libvirtd uses what's ostensible a
public, supported interface. That means when we add things like this,
we're stuck supporting it for general use cases.

It's much more palatable to do these things using a private interface
such that we can change these things down the road without worrying
about compatibility with third-party tools.

Regards,

Anthony Liguori



Is this a nack for the fd: protocol?  Or do we want to implement the fd: 
protocol as a stepping stone on the way to a privilege-separated qemu 
model?  I know the fd: protocol is not ideal, but it does provide NFS 
image isolation, perhaps much sooner than privilege-separated qemu can.


--
Regards,
Corey

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

Re: [libvirt] [PATCH] qemu: Report error if qemu monitor command not found for BlockJob

[Adam Litke]

On 08/22/2011 10:04 PM, Osier Yang wrote:
 Hi, Adam
 
 I likes the idea to wrap the checking as a function seperately, but the
 function
 won't work well if the command is help info, though we don't have a use
 of help info yet.
 
 My point is since the function is intending to work for all the command,
 as a
 common function, it needs to consider some boudary too.
 
 How about below?

Good points.  The function below looks good.

 qemuMonitorTextCommandNotFound(const char *cmd, const char *reply)
 {
 if (STRPREFIX(cmd, info )) {
 if (strstr(reply, info version))
 return 1;
 } else {
 if (strstr(reply, unknown command:))
 return 1;
 }
 
 return 0;
 }
 
 And there might be other different info qemu will output for a unknown
 command
 we don't known yet. Using cmd as an argument will allow us to extend
 the checking
 methods.
 
 Thanks
 Osier

-- 
Adam Litke
IBM Linux Technology Center

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Anthony Liguori]

On 08/23/2011 09:26 AM, Corey Bryant wrote:

On 08/22/2011 03:25 PM, Anthony Liguori wrote:

On 08/22/2011 01:22 PM, Daniel P. Berrange wrote:

On Mon, Aug 22, 2011 at 12:25:25PM -0500, Anthony Liguori wrote:

On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:

On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:

I don't think it makes sense to have qemu-fe do dynamic labelling.
You certainly could avoid the fd passing by having qemu-fe do the
open though and just let qemu-fe run without the restricted security
context.


qemu-fe would also not be entirely simple,


Indeed.


because it will need to act
as a proxy for the monitor, in order to make hotplug work. ie the mgmt
app would be sending 'drive_add file:/foo/bar' to qemu-fe, which would
then have to open the file and send 'drive_add fd:NN' onto the real
QEMU,
and then pass the results on back.

In addition qemu-fe would still have to be under some kind of
restricted
security context for it to be acceptable. This is going to want to
be as
locked down as possible.


I think there's got to be some give and take here.

It should at least be as locked down as libvirtd. From a security
point of view, we should be able to agree that we want libvirtd to
be as locked down as possible.

But there shouldn't be a hard requirement to lock down qemu-fe more
than libvirtd. Instead, the requirement should be for qemu-fe to be
as/more vigilant in not trusting qemu-system-x86_64 as libvirtd is.

The fundamental problem here, is that there is some logic in
libvirtd that rightly belongs in QEMU. In order to preserve the
security model, that means that we're going to have to take a
subsection of QEMU and trust it more.


Well we have a process that makes security decisions, and a process
which applies those security decisions and a process which is confined
by those decisions. Currently libvirtd makes applies the decisions,
and qemu is confined. A qemu-fe model would mean that libvirt is making
the decisions, but is then relying on qemu-fe to apply them. IMHO that
split is undesirable, but that's besides the point, since this is not
a decision that needs to be made now.

'qemu-fe' needs to have a way to communicate with the confined process
('qemu-system-XXX') to supply it the resources (file FDs) it needs to
access. The requirements of such a comms channel for qemu-fe are going
to be the same as those needed by libvirtd talking to QEMU today, or
indeed by any process that is applying security decisions to QEMU.


But the fundamental difference is that libvirtd uses what's ostensible a
public, supported interface. That means when we add things like this,
we're stuck supporting it for general use cases.

It's much more palatable to do these things using a private interface
such that we can change these things down the road without worrying
about compatibility with third-party tools.

Regards,

Anthony Liguori



Is this a nack for the fd: protocol?


No, I think we're trying to understand what the options are.

Regards,

Anthony Liguori

 Or do we want to implement the fd:

protocol as a stepping stone on the way to a privilege-separated qemu
model? I know the fd: protocol is not ideal, but it does provide NFS
image isolation, perhaps much sooner than privilege-separated qemu can.



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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Stefan Hajnoczi]

On Tue, Aug 23, 2011 at 2:48 PM, Eric Blake ebl...@redhat.com wrote:
 On 08/23/2011 04:28 AM, Stefan Hajnoczi wrote:

 I forgot to ask the obvious question:

 I am writing a backup program that is using the new snapshot APIs.  A
 snapshot has been created, how do I read out the data from the
 snapshot?

 Here's how to access the data in the snapshot, at least for the first round
 implementation of qcow2 snapshots:

 If you created an internal snapshot (virDomainSnapshotCreateXML with no
 flags), then the only way right now to read data out is to shut down any
 qemu process (since qemu-img should not be used on a file in active use by
 qemu), then:
 qemu-img convert [options] -s snapshot file backup
 to extract the named internal snapshot from 'file' into a new file 'backup'.

 If you created an external snapshot (virDomainSnapshotCreateXML with the new
 _DISK_ONLY flag), then the data from the snapshot is the old file name.
  That is, if you start with '/path/to/old', then create a snapshot with a
 target file of '/path/to/new', then /path/to/old _is_ the snapshot, and
 /path/to/new is a qcow2 file with /path/to/old as its backing file.  The
 snapshot (old file) can safely be accessed even while qemu is still running.

Hmm...so there is no abstraction.  I still need to understand what the
underlying snapshot implementation does and what its limitations are.

This is what I meant when I asked about aiming for something more
high-level where the user doesn't need to be an expert in snapshots to
use this API.  In order to access the snapshot I need to use an
out-of-band (ssh?) mechanism to get to the libvirt host and know how
to access the external snapshot image file.  If that image file is
using an image format then I need to use libguestfs, qemu-io/qemu-img,
or custom code to access the format.

I think we simply cannot expose all this complexity to users.  Each
application would have to support the many different cases.  Libvirt
needs to tie this stuff together and present an interface that
applications can use without worrying how to actually get at the
snapshot data.

Stefan

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

Re: [libvirt] [PATCH 34/26] snapshot: add disks to snapshot xml

[Eric Blake]

On 08/19/2011 03:58 PM, Eric Blake wrote:

Adds an optional element todomainsnapshot, which will be used
to give user control over external snapshot filenames on input,
and specify generated filenames on output.


Another couple of problems.


+static int
+disksorter(const void *a, const void *b)
+{
+const virDomainSnapshotDiskDef *diska = a;
+const virDomainSnapshotDiskDef *diskb = b;
+
+return diskb-index - diska-index;
+}


Backwards.  Should be a - b.



+
+/* Provide defaults for all remaining disks.  */
+if (VIR_EXPAND_N(def-disks, def-ndisks,
+ def-dom-ndisks - def-ndisks)  0) {


This updates def-ndisks,


+virReportOOMError();
+goto cleanup;
+}
+
+for (i = 0; i  def-dom-ndisks; i++) {
+virDomainSnapshotDiskDefPtr disk;
+
+ignore_value(virBitmapGetBit(map, i,inuse));
+if (inuse)
+continue;
+disk =def-disks[def-ndisks++];


and this ends up accessing beyond array bounds.


+ignore_value(virAsprintf(disk-file, %*s%s,
+ (int) (tmp - original), original,
+ def-name));


Needs to be %.*s, not %*s, in order to truncate correctly.

I'll just post a v3 of this patch, instead of adding to the squash list.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Stefan Hajnoczi]

On Tue, Aug 23, 2011 at 2:40 PM, Eric Blake ebl...@redhat.com wrote:
 On 08/23/2011 03:52 AM, Stefan Hajnoczi wrote:

 On Wed, Aug 10, 2011 at 11:08 PM, Eric Blakeebl...@redhat.com  wrote:

 disk snapshot: the state of a virtual disk used at a given time; once a
 snapshot exists, then it is possible to track a delta of changes that
 have
 happened since that time.

 Did you go into details of the delta API anywhere?  I don't see it.

 It's not available yet, because qemu doesn't provide anything yet.  I think
 that APIs to inspect the actual delta disk contents between the current
 state and a prior snapshot will be similar to block pull, but we can't
 implement anything without support from the underlying tools.

Excellent, this is an opportunity where we need to think things
through on the QEMU side and come up with a proposal that you can give
feedback on.

There is no active work in implementing a dirty block tracking API in QEMU.

We already have the bs-dirty_bitmap for block-migration.c.  Jagane
has also implemented a dirty block feature for his LiveBackup API:
https://github.com/jagane/qemu-livebackup/blob/master/livebackup.h#L95

We also have the actual bdrv_is_allocated() information to determine
whether a qcow2/qed/etc image file has the sectors allocated or not.

As a starting point we could provide a way to enable bs-dirty_bitmap
for a block device and query its status.  This is not persistent (the
bitmap is in RAM) so the question becomes whether or not to persist?
And if we persist do we want to take the cheap route of syncing the
bitmap to disk only when cleanly terminating QEMU or to do a
crash-safe bitmap?

If we specify that the dirty bitmap is not guaranteed to persist
(because it is simply an advisory feature for incremental backup and
similar applications), then we can start simple and consider doing a
persistent implementation later.

Stefan

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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Eric Blake]

On 08/23/2011 09:12 AM, Stefan Hajnoczi wrote:

Hmm...so there is no abstraction.  I still need to understand what the
underlying snapshot implementation does and what its limitations are.


We're still trying to get to that point.  A while ago, I posted an RFC 
for adding virStorageVolSnapshot* APIs, which would be the ideal place 
to expose storage-volume independent wrappers around snapshot 
management.  The idea is still that we can use APIs like that (instead 
of low-level access to the snapshot image file) to stream snapshot data 
from a remote host back to the client.  We also need a new API that lets 
you quickly access all of the storage volumes associated with a domain 
(my patch to add 'virsh domblklist' is a start at getting at all the 
storage volume names, but not quite as good as getting the actual 
virStorageVolPtr objects).




This is what I meant when I asked about aiming for something more
high-level where the user doesn't need to be an expert in snapshots to
use this API.  In order to access the snapshot I need to use an
out-of-band (ssh?) mechanism to get to the libvirt host and know how
to access the external snapshot image file.  If that image file is
using an image format then I need to use libguestfs, qemu-io/qemu-img,
or custom code to access the format.


For now, yes.  But hopefully the work I'm doing is providing enough 
framework to later add the additions that can indeed expose libvirt APIs 
rather than low-level tool knowledge to get at the snapshots.




I think we simply cannot expose all this complexity to users.  Each
application would have to support the many different cases.  Libvirt
needs to tie this stuff together and present an interface that
applications can use without worrying how to actually get at the
snapshot data.


I don't see any problem with exposing the lower layers as a start, then 
adding higher layers as we go.  There are different classes of users, 
and both layers are useful in the right context.  But at the same time, 
I agree with you that what I have done so far is just a start, and by no 
means the end of snapshot-related libvirt work.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Corey Bryant]

On 08/22/2011 02:39 PM, Blue Swirl wrote:

On Mon, Aug 22, 2011 at 5:42 PM, Corey Bryantcor...@linux.vnet.ibm.com  wrote:



  On 08/22/2011 01:25 PM, Anthony Liguori wrote:


  On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:


  On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:


  I don't think it makes sense to have qemu-fe do dynamic labelling.
  You certainly could avoid the fd passing by having qemu-fe do the
  open though and just let qemu-fe run without the restricted security
  context.


  qemu-fe would also not be entirely simple,


  Indeed.



  I do like the idea of a privileged qemu-fe performing the open and passing
  the fd to a restricted qemu.

Me too.


However, I get the impression that this won't
  get delivered nearly as quickly as fd: passing could be.  How soon do we
  need image isolation for NFS?

  Btw, this sounds similar to what Blue Swirl recommended here on v1 of this
  patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html

I was thinking about simply doing fork() + setuid() at some point and
using the FD passing structures directly. But would it bring
advantages to have two separate executables, are they different from
access control point of view vs. single but forked one?



We could put together an SELinux policy that would transition qemu-fe to 
a more restricted domain (ie. no open privilege on NFS files) when it 
executes qemu-system-x86_64.


--
Regards,
Corey


  Regards,
  Corey


  because it will need to act
  as a proxy for the monitor, in order to make hotplug work. ie the mgmt
  app would be sending 'drive_addfile:/foo/bar' to qemu-fe, which would
  then have to open the file and send 'drive_add fd:NN' onto the real QEMU,
  and then pass the results on back.

  In addition qemu-fe would still have to be under some kind of restricted
  security context for it to be acceptable. This is going to want to be as
  locked down as possible.


  I think there's got to be some give and take here.

  It should at least be as locked down as libvirtd. From a security point
  of view, we should be able to agree that we want libvirtd to be as
  locked down as possible.

  But there shouldn't be a hard requirement to lock down qemu-fe more than
  libvirtd. Instead, the requirement should be for qemu-fe to be as/more
  vigilant in not trusting qemu-system-x86_64 as libvirtd is.

  The fundamental problem here, is that there is some logic in libvirtd
  that rightly belongs in QEMU. In order to preserve the security model,
  that means that we're going to have to take a subsection of QEMU and
  trust it more.


  So I'd see that you'd likely end up with the
  qemu-fe security policy being identical to the qemu security policy,


  Then there's no point in doing qemu-fe. qemu-fe should be thought of as
  QEMU supplied libvirtd plugin.


  with the exception that it would be allowed to open files on NFS without
  needing them to be labelled. So I don't really see that all this gives us
  any tangible benefits over just allowing the mgmt app to pass in the FDs
  directly.


  But libvirt would still need to parse image files.


  Not neccessarily. As mentioned below, it is entirely possible to
  enable the mgmt app to pass in details of the backing files, at
  which point no image parsing is required by libvirt. Hence my
  assertion that the question of who does image parsing is irrelevant
  to this discussion.


  That's certainly true.

  Regards,

  Anthony Liguori






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

Re: [libvirt] Notes from the KVM Forum relevant to libvirt

[Stefan Hajnoczi]

On Tue, Aug 23, 2011 at 12:15 PM, Daniel P. Berrange
berra...@redhat.com wrote:
 I was at the KVM Forum / LinuxCon last week and there were many
 interesting things discussed which are relevant to ongoing libvirt
 development. Here was the list that caught my attention. If I have
 missed any, fill in the gaps

  - Sandbox/container KVM.  The Solaris port of KVM puts QEMU inside
   a zone so that an exploit of QEMU can't escape into the full OS.
   Containers are Linux's parallel of Zones, and while not nearly as
   secure yet, it would still be worth using more containers support
   to confine QEMU.

Can you elaborate on why Linux containers are not nearly as secure
[as Solaris Zones]?

Containers is just another attempt at isolating the QEMU process.
SELinux works differently but can also do many of the same things.  I
like containers more because they are simpler than labelling
everything.

  - Native KVM tool. The problem statement was that the QEMU code is too
   big/complex  and command line args are too complex, so lets rewrite
   from scratch to make the code small  CLI simple. They achieve this,
   but of course primarily because they lack so many features compared
   to QEMU. They had libvirt support as a bullet point on their preso,
   but I'm not expecting it to replace the current QEMU KVM support in
   the forseeable future, given its current level of features and the
   size of its dev team compared to QEMU/KVM. They did have some fun
   demos of booting using the host OS filesystem though. We can
   actually do the same with regular KVM/libvirt but there's no nice
   demo tool to show it off. I'm hoping to create one

Yep it's virtfs which QEMU has supported for a while.  The trick is
setting things up so that the Linux guest boots from virtfs.

Stefan

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 11:13:34AM -0400, Corey Bryant wrote:
 
 
 On 08/22/2011 02:39 PM, Blue Swirl wrote:
 On Mon, Aug 22, 2011 at 5:42 PM, Corey Bryantcor...@linux.vnet.ibm.com  
 wrote:
 
 
   On 08/22/2011 01:25 PM, Anthony Liguori wrote:
 
   On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:
 
   On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:
 
   I don't think it makes sense to have qemu-fe do dynamic labelling.
   You certainly could avoid the fd passing by having qemu-fe do the
   open though and just let qemu-fe run without the restricted 
  security
   context.
 
   qemu-fe would also not be entirely simple,
 
   Indeed.
 
 
   I do like the idea of a privileged qemu-fe performing the open and 
  passing
   the fd to a restricted qemu.
 Me too.
 
 However, I get the impression that this won't
   get delivered nearly as quickly as fd: passing could be.  How soon do we
   need image isolation for NFS?
 
   Btw, this sounds similar to what Blue Swirl recommended here on v1 of 
  this
   patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html
 I was thinking about simply doing fork() + setuid() at some point and
 using the FD passing structures directly. But would it bring
 advantages to have two separate executables, are they different from
 access control point of view vs. single but forked one?
 
 
 We could put together an SELinux policy that would transition
 qemu-fe to a more restricted domain (ie. no open privilege on NFS
 files) when it executes qemu-system-x86_64.

Thinking about this some more, I don't really think the idea of delegating
open of NFS files to a separate qemu-fe is very desirable. Libvirt makes the
decision on the security policy that the VM will run under, and provides
audit records to log what resources are being assigned to the VM. From that
point onwards, we must be able to guarantee that MAC will be enforced on
the VM, according to what we logged via the auditd system.

In the case where we delegate opening of the files to qemu-fe, and allow
its policy to open NFS files, we no longer have a guarentee that the MAC
policy will be enforced as we originally intended. Yes, qemu-fe will very
likely honour what we tell it and open the correct files, and yes qmeu-fe
has lower attack surface wrt the guest than the real qemu does, but we
still loose the guarentee of MAC enforcement from libvirt's POV.

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] Notes from the KVM Forum relevant to libvirt

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 04:24:46PM +0100, Stefan Hajnoczi wrote:
 On Tue, Aug 23, 2011 at 12:15 PM, Daniel P. Berrange
 berra...@redhat.com wrote:
  I was at the KVM Forum / LinuxCon last week and there were many
  interesting things discussed which are relevant to ongoing libvirt
  development. Here was the list that caught my attention. If I have
  missed any, fill in the gaps
 
   - Sandbox/container KVM.  The Solaris port of KVM puts QEMU inside
    a zone so that an exploit of QEMU can't escape into the full OS.
    Containers are Linux's parallel of Zones, and while not nearly as
    secure yet, it would still be worth using more containers support
    to confine QEMU.
 
 Can you elaborate on why Linux containers are not nearly as secure
 [as Solaris Zones]?

Mostly because the Linux namespace functionality is far from complete,
notably lacking proper UID/GID/capability separation, and UID/GID
virtualization wrt filesystems. The longer answer is here:

   https://wiki.ubuntu.com/UserNamespace

So at this time you can't build a secure container on Linux, relying
just on DAC alone. You have to add in a MAC layer ontop of the container
to get full security benefits, which obviously defeats the point of
using the container as a backup for failure in the MAC layer.

   - Native KVM tool. The problem statement was that the QEMU code is too
    big/complex  and command line args are too complex, so lets rewrite
    from scratch to make the code small  CLI simple. They achieve this,
    but of course primarily because they lack so many features compared
    to QEMU. They had libvirt support as a bullet point on their preso,
    but I'm not expecting it to replace the current QEMU KVM support in
    the forseeable future, given its current level of features and the
    size of its dev team compared to QEMU/KVM. They did have some fun
    demos of booting using the host OS filesystem though. We can
    actually do the same with regular KVM/libvirt but there's no nice
    demo tool to show it off. I'm hoping to create one
 
 Yep it's virtfs which QEMU has supported for a while.  The trick is
 setting things up so that the Linux guest boots from virtfs.

It isn't actually that hard from a technical POV, it is just that most
(all?) distros typical  initrd files lack support for specifying 9p over
virtio as a root filesystem.

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] [RFC 01/12] Add various USB devices QEMU_CAPS

[Daniel P. Berrange]

On Sun, Aug 21, 2011 at 10:01:12PM +0300, Marc-André Lureau wrote:
 ---
  src/qemu/qemu_capabilities.c |   31 +++
  src/qemu/qemu_capabilities.h |   10 ++
  tests/qemuhelptest.c |   13 ++---
  3 files changed, 51 insertions(+), 3 deletions(-)
 
 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index f665de4..a33ad4e 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -125,6 +125,17 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
sga,
virtio-blk-pci.event_idx,
virtio-net-pci.event_idx,
 +
 +  piix3-usb-uhci, /* 65 */
 +  piix4-usb-uhci,
 +  usb-ehci,
 +  ich9-usb-ehci1,
 +  ich9-usb-uhci1,
 +
 +  ich9-usb-uhci2, /* 70 */
 +  ich9-usb-uhci3,
 +  vt82c686b-usb-uhci,
 +  usb-redir,
  );

IMHO this is a little bit overkill. If bunch of things were introduced
at the same time there is no need to probe for each one of them. ie
you'll never find that you have 'ich9-usb-echi', but not also have
'ich9-usb-uhci1,2,3', so probing for all 4 individually just bloats
our capabilities.

Likewise we don't generally bother probing for things which have been
around since 0.9.0 (our min version) ie the piix USB controllers.

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] RFCv2: virDomainSnapshotCreateXML enhancements

[Stefan Hajnoczi]

On Tue, Aug 23, 2011 at 4:18 PM, Eric Blake ebl...@redhat.com wrote:
 On 08/23/2011 09:12 AM, Stefan Hajnoczi wrote:
 I think we simply cannot expose all this complexity to users.  Each
 application would have to support the many different cases.  Libvirt
 needs to tie this stuff together and present an interface that
 applications can use without worrying how to actually get at the
 snapshot data.

 I don't see any problem with exposing the lower layers as a start, then
 adding higher layers as we go.  There are different classes of users, and
 both layers are useful in the right context.  But at the same time, I agree
 with you that what I have done so far is just a start, and by no means the
 end of snapshot-related libvirt work.

Do you have a user in mind who will be able to use this API?  The
kinds of apps I am thinking about cannot make use of this API.

This is largely because there is no API for accessing snapshot
contents.  But even the snapshot API itself has too many flags/cases
that require the user to already know exactly what they want to do
down to a level of detail where I wonder why they would even want to
use libvirt and not just do, say, LVM snapshots manually.

Perhaps I'm missing what adding this API enables.  Please share what
use case you have in mind.

Stefan

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

Re: [libvirt] [RFC 05/12] USB devices gain a new USB address child element

[Daniel P. Berrange]

On Sun, Aug 21, 2011 at 10:01:16PM +0300, Marc-André Lureau wrote:
 ---
  docs/schemas/domain.rng|   14 +
  src/conf/domain_conf.c |   62 
 +++-
  src/conf/domain_conf.h |   10 +++
  src/qemu/qemu_command.c|   40 ++---
  src/qemu/qemu_command.h|6 +-
  src/qemu/qemu_hotplug.c|2 +-
  .../qemuxml2argv-input-usbmouse-addr.args  |1 +
  .../qemuxml2argv-input-usbmouse-addr.xml   |   27 +
  tests/qemuxml2argvtest.c   |2 +
  9 files changed, 149 insertions(+), 15 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml

So this lets users specify an explicit USB address for their devices.

If an existing app is using libvirt though, they won't be doing this,
and we want to make sure they get stable USB port addresses. With PCI
we have a function in the QEMU driver which will assign PCI slots to
any PCI device which doesn't already have one. I think we need todo
the same for USB ports, so all configs become migration-safe.


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] [RFC 03/12] Add a new controller type 'usb' with optionnal 'model'

[Daniel P. Berrange]

On Sun, Aug 21, 2011 at 10:01:14PM +0300, Marc-André Lureau wrote:
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index dbfc7d9..4168504 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -83,6 +83,24 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
, /* don't support vbox */
qxl);
  
 +VIR_ENUM_DECL(qemuControllerModel)
 +
 +VIR_ENUM_IMPL(qemuControllerModel, VIR_DOMAIN_CONTROLLER_MODEL_LAST,
 +  , /* auto */
 +  , /* buslogic don't support */
 +  , /* lsilogic don't support */
 +  , /* lsisas don't support */
 +  , /* vmpvscsi don't support */
 +  piix3-usb-uhci,
 +  piix4-usb-uhci,
 +  usb-ehci,
 +  ich9-usb-ehci1,
 +  ich9-usb-uhci1,
 +  ich9-usb-uhci2,
 +  ich9-usb-uhci3,
 +  vt82c686b-usb-uhci);

If we separate out the model enums per controller type, then we
can avoid the nasty hacks of  in this declaration.

 +static int
 +qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
 + virBitmapPtr qemuCaps,
 + virBuffer *buf)
 +{
 +const char *smodel;
 +int model, caps;
 +
 +model = def-model;
 +if (model == -1 || model == VIR_DOMAIN_CONTROLLER_MODEL_AUTO)
 +model = VIR_DOMAIN_CONTROLLER_MODEL_PIIX3_UHCI;
 +
 +smodel = qemuControllerModelTypeToString(model);
 +caps = qemuControllerModelToCaps(model);
 +
 +if (caps == -1 || !qemuCapsGet(qemuCaps, caps)) {
 +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(%s not supported in this QEMU binary), smodel);
 +return -1;
 +}
 +
 +virBufferAsprintf(buf, %s,id=usb%d, smodel, def-idx);
 +return 0;
 +}
 +


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] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Kevin Wolf]

Am 23.08.2011 17:26, schrieb Daniel P. Berrange:
 On Tue, Aug 23, 2011 at 11:13:34AM -0400, Corey Bryant wrote:


 On 08/22/2011 02:39 PM, Blue Swirl wrote:
 On Mon, Aug 22, 2011 at 5:42 PM, Corey Bryantcor...@linux.vnet.ibm.com  
 wrote:


  On 08/22/2011 01:25 PM, Anthony Liguori wrote:

  On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:

  On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:

  I don't think it makes sense to have qemu-fe do dynamic labelling.
  You certainly could avoid the fd passing by having qemu-fe do the
  open though and just let qemu-fe run without the restricted 
 security
  context.

  qemu-fe would also not be entirely simple,

  Indeed.


  I do like the idea of a privileged qemu-fe performing the open and 
 passing
  the fd to a restricted qemu.
 Me too.

However, I get the impression that this won't
  get delivered nearly as quickly as fd: passing could be.  How soon do we
  need image isolation for NFS?

  Btw, this sounds similar to what Blue Swirl recommended here on v1 of 
 this
  patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html
 I was thinking about simply doing fork() + setuid() at some point and
 using the FD passing structures directly. But would it bring
 advantages to have two separate executables, are they different from
 access control point of view vs. single but forked one?


 We could put together an SELinux policy that would transition
 qemu-fe to a more restricted domain (ie. no open privilege on NFS
 files) when it executes qemu-system-x86_64.
 
 Thinking about this some more, I don't really think the idea of delegating
 open of NFS files to a separate qemu-fe is very desirable. Libvirt makes the
 decision on the security policy that the VM will run under, and provides
 audit records to log what resources are being assigned to the VM. From that
 point onwards, we must be able to guarantee that MAC will be enforced on
 the VM, according to what we logged via the auditd system.
 
 In the case where we delegate opening of the files to qemu-fe, and allow
 its policy to open NFS files, we no longer have a guarentee that the MAC
 policy will be enforced as we originally intended. Yes, qemu-fe will very
 likely honour what we tell it and open the correct files, and yes qmeu-fe
 has lower attack surface wrt the guest than the real qemu does, but we
 still loose the guarentee of MAC enforcement from libvirt's POV.

On the other hand, from a qemu POV libvirt is only one possible
management tool. In practice, another very popular management tool for
qemu is bash. With qemu-fe all the other tools, including direct
invocation from the command line, would get some protection, too.

Kevin

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

Re: [libvirt] RFCv2: virDomainSnapshotCreateXML enhancements

[Eric Blake]

On 08/23/2011 09:35 AM, Stefan Hajnoczi wrote:

On Tue, Aug 23, 2011 at 4:18 PM, Eric Blakeebl...@redhat.com  wrote:

On 08/23/2011 09:12 AM, Stefan Hajnoczi wrote:

I think we simply cannot expose all this complexity to users.  Each
application would have to support the many different cases.  Libvirt
needs to tie this stuff together and present an interface that
applications can use without worrying how to actually get at the
snapshot data.


I don't see any problem with exposing the lower layers as a start, then
adding higher layers as we go.  There are different classes of users, and
both layers are useful in the right context.  But at the same time, I agree
with you that what I have done so far is just a start, and by no means the
end of snapshot-related libvirt work.


Do you have a user in mind who will be able to use this API?


Yes.  Someone who wants to do a local device migration can do:

start with a domain with a disk backed only by local storage

virDomainSnapshotCreateXML(,DISK_ONLY) with XML that directs qemu to 
convert that disk over to a qcow2 image on shared storage


virDomainBlockPull to copy all the contents of the local file into the 
shared copy


migrate, now that the domain is no longer tied to the local device

delete the snapshot as it is no longer needed



Also, as I have been implementing the series, I have been playing with 
creating snapshots then reverting to them.  This works reliably (I am 
indeed able to rewind disk state), which means it should not be much 
longer in my series before I am able to implement a transient/ disk 
property for qemu, which auto-rewinds disk state at domain exit.  That 
is, getting the low-level snapshot support working is a stepping stone 
towards several useful features, even if the feature that you are asking 
for, which is remote access to the actual contents of the snapshot, 
still requires third-party tools at the moment rather than being exposed 
via higher-layer libvirt APIs.



 The
kinds of apps I am thinking about cannot make use of this API.


What sort of APIs do you need for the apps you are thinking about? 
Without details of your needs, it's hard to say whether we can build on 
the framework we have to add the additional features you want.




This is largely because there is no API for accessing snapshot
contents.  But even the snapshot API itself has too many flags/cases
that require the user to already know exactly what they want to do
down to a level of detail where I wonder why they would even want to
use libvirt and not just do, say, LVM snapshots manually.


You _can't_ make a manual LVM snapshot of a running qemu process, and 
expect the result to be consistent.  But you _can_ use my new API to 
create an external qcow2 snapshot, at which point you can then access 
the backing file and create an LVM snapshot.  That is, the goal of this 
API series right now is to add live external snapshot support - exposing 
the qemu snapshot_blkdev monitor command - while still leaving the xml 
flexible enough to later add further snapshot capabilities.




Perhaps I'm missing what adding this API enables.  Please share what
use case you have in mind.

Stefan



--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 05:50:03PM +0200, Kevin Wolf wrote:
 Am 23.08.2011 17:26, schrieb Daniel P. Berrange:
  On Tue, Aug 23, 2011 at 11:13:34AM -0400, Corey Bryant wrote:
 
 
  On 08/22/2011 02:39 PM, Blue Swirl wrote:
  On Mon, Aug 22, 2011 at 5:42 PM, Corey Bryantcor...@linux.vnet.ibm.com  
  wrote:
 
 
   On 08/22/2011 01:25 PM, Anthony Liguori wrote:
 
   On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:
 
   On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:
 
   I don't think it makes sense to have qemu-fe do dynamic 
  labelling.
   You certainly could avoid the fd passing by having qemu-fe do the
   open though and just let qemu-fe run without the restricted 
  security
   context.
 
   qemu-fe would also not be entirely simple,
 
   Indeed.
 
 
   I do like the idea of a privileged qemu-fe performing the open and 
  passing
   the fd to a restricted qemu.
  Me too.
 
 However, I get the impression that this won't
   get delivered nearly as quickly as fd: passing could be.  How soon do 
  we
   need image isolation for NFS?
 
   Btw, this sounds similar to what Blue Swirl recommended here on v1 of 
  this
   
  patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html
  I was thinking about simply doing fork() + setuid() at some point and
  using the FD passing structures directly. But would it bring
  advantages to have two separate executables, are they different from
  access control point of view vs. single but forked one?
 
 
  We could put together an SELinux policy that would transition
  qemu-fe to a more restricted domain (ie. no open privilege on NFS
  files) when it executes qemu-system-x86_64.
  
  Thinking about this some more, I don't really think the idea of delegating
  open of NFS files to a separate qemu-fe is very desirable. Libvirt makes the
  decision on the security policy that the VM will run under, and provides
  audit records to log what resources are being assigned to the VM. From that
  point onwards, we must be able to guarantee that MAC will be enforced on
  the VM, according to what we logged via the auditd system.
  
  In the case where we delegate opening of the files to qemu-fe, and allow
  its policy to open NFS files, we no longer have a guarentee that the MAC
  policy will be enforced as we originally intended. Yes, qemu-fe will very
  likely honour what we tell it and open the correct files, and yes qmeu-fe
  has lower attack surface wrt the guest than the real qemu does, but we
  still loose the guarentee of MAC enforcement from libvirt's POV.
 
 On the other hand, from a qemu POV libvirt is only one possible
 management tool. In practice, another very popular management tool for
 qemu is bash. With qemu-fe all the other tools, including direct
 invocation from the command line, would get some protection, too.

That's why I said a qemu-fe like tool need not be mutually exclusive
with exposing FD passing to a management tool like libvirt. Both
qemu-fe and libvirt need to some mechanism to pass open FDs to the
real QEMU.  We could needlessly invent a new communication channel
between qemu-fe and qemu that only it can use, or we can use the
channel we already have - QMP - to provide an interface that either
qemu-fe or libvirt, can use to pass FDs into the real QEMU.

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/3] daemon: Create priority workers pool

[Michal Privoznik]

On 23.08.2011 14:23, Daniel P. Berrange wrote:
 On Tue, Aug 16, 2011 at 06:39:10PM +0200, Michal Privoznik wrote:
 This patch annotates APIs with low or high priority.
 In low set MUST be all APIs which might eventually access monitor
 (and thus block indefinitely). Other APIs may be marked as high
 priority. However, some must be (e.g. domainDestroy).

 For high priority calls (HPC), there is new thread pool created.
 HPC tries to land in usual thread pool firstly. The condition here
 is it contains at least one free worker. As soon as it doesn't,
 HPCs are placed into the new pool. Therefore, only those APIs which
 are guaranteed to end in reasonable small amount of time can be marked
 as HPC.

 The size of this HPC pool is static, because HPC are expected to end
 quickly, therefore jobs assigned to this pool will be served quickly.
 It can be configured in libvirtd.conf via prio_workers variable.
 Default is set to 5.

 To mark API with low or high priority, append priority:{low|high} to
 it's comment in src/remote/remote_protocol.x. This is similar to
 autogen|skipgen.
 
 
 
 diff --git a/daemon/remote.c b/daemon/remote.c
 index 939044c..f6c6f35 100644
 --- a/daemon/remote.c
 +++ b/daemon/remote.c
 @@ -464,6 +464,32 @@ int remoteClientInitHook(virNetServerPtr srv 
 ATTRIBUTE_UNUSED,
  return 0;
  }
  
 +int remoteGetProcPriority(virNetMessageHeaderPtr hdr)
 +{
 +u_int prog = hdr-prog;
 +int proc = hdr-proc;
 +int *table = NULL;
 +size_t max = 0;
 +
 +switch (prog) {
 +case REMOTE_PROGRAM:
 +table = remoteProcsPriority;
 +max = remoteNProcs;
 +break;
 +case QEMU_PROGRAM:
 +table = qemuProcsPriority;
 +max = qemuNProcs;
 +break;
 +}
 +
 +if (!table || !max)
 +return 0;
 +if (proc = max)
 +return 0;
 +
 +return table[proc];
 +}
 
 
 I don't think this is the right way provide the priority information.
 We already have a extensible table for information about procedures
 in virnetserverprogram.h
 
 struct _virNetServerProgramProc {
 virNetServerProgramDispatchFunc func;
 size_t arg_len;
 xdrproc_t arg_filter;
 size_t ret_len;
 xdrproc_t ret_filter;
 bool needAuth;
 };
 
 
 We should be adding 'unsigned int priority' to the struct.
 
 
 diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
 index 0d344e8..a8f9fc6 100755
 --- a/src/rpc/gendispatch.pl
 +++ b/src/rpc/gendispatch.pl
 @@ -954,6 +970,28 @@ elsif ($opt_b) {
  }
  print };\n;
  print size_t ${structprefix}NProcs = 
 ARRAY_CARDINALITY(${structprefix}Procs);\n;
 +
 +# Now we write priority table
 +
 +print \nint ${structprefix}ProcsPriority[] = {\n;
 +for ($id = 0 ; $id = $#calls ; $id++) {
 +if (!defined $calls[$id]) {
 +print 0, /* Unkown proc $id */\n;
 +next;
 +}
 +if ($calls[$id]-{priority}) {
 +print 1;
 +} else {
 +print 0;
 +}
 +if ($calls[$id]-{UC_NAME}) {
 +print , /*  ${procprefix}_PROC_$calls[$id]-{UC_NAME} */;
 +} else {
 +print ,;
 +}
 +print \n;
 +}
 +print };\n;
  }
 
 And so instead of creating a new table here, we should just be writing
 the new priority field to the existing table.
 
  # Bodies for client functions (remote_client_bodies.h).
 diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
 index 1a49dbb..b8150b7 100644
 --- a/src/rpc/virnetserver.c
 +++ b/src/rpc/virnetserver.c
 @@ -72,9 +72,12 @@ struct _virNetServer {
  virMutex lock;
  
  virThreadPoolPtr workers;
 +virThreadPoolPtr prio_workers;
  
  bool privileged;
  
 +virNetServerPriorityProcFunc prio_func;
 +
  size_t nsignals;
  virNetServerSignalPtr *signals;
  int sigread;
 @@ -182,6 +185,7 @@ static int 
 virNetServerDispatchNewMessage(virNetServerClientPtr client,
  {
  virNetServerPtr srv = opaque;
  virNetServerJobPtr job;
 +bool priority = false;
  int ret;
  
  VIR_DEBUG(server=%p client=%p message=%p,
 @@ -192,11 +196,25 @@ static int 
 virNetServerDispatchNewMessage(virNetServerClientPtr client,
  return -1;
  }
  
 +if (srv-prio_func)
 +priority = srv-prio_func(msg-header);
 +
  job-client = client;
  job-msg = msg;
  
  virNetServerLock(srv);
 -if ((ret = virThreadPoolSendJob(srv-workers, job))  0)
 +ret = virThreadPoolSendJob(srv-workers, priority, job);
 +if (ret  -1) {
 +goto cleanup;
 +} else if (ret == -1) {
 +/* try placing job into priority pool */
 +VIR_DEBUG(worker pool full, placing proc %d into priority pool,
 +  msg-header.proc);
 +ret = virThreadPoolSendJob(srv-prio_workers, false, job);
 +}
 +
 +cleanup:
 +if (ret  0)
  VIR_FREE(job);
  virNetServerUnlock(srv);
  
 
 One thing that concerns me is that using 2 separate 

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Daniel P. Berrange]

On Tue, Aug 23, 2011 at 04:51:31PM +0100, Daniel P. Berrange wrote:
 On Tue, Aug 23, 2011 at 05:50:03PM +0200, Kevin Wolf wrote:
  Am 23.08.2011 17:26, schrieb Daniel P. Berrange:
   On Tue, Aug 23, 2011 at 11:13:34AM -0400, Corey Bryant wrote:
  
  
   On 08/22/2011 02:39 PM, Blue Swirl wrote:
   On Mon, Aug 22, 2011 at 5:42 PM, Corey 
   Bryantcor...@linux.vnet.ibm.com  wrote:
  
  
On 08/22/2011 01:25 PM, Anthony Liguori wrote:
  
On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:
  
On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:
  
I don't think it makes sense to have qemu-fe do dynamic 
   labelling.
You certainly could avoid the fd passing by having qemu-fe do 
   the
open though and just let qemu-fe run without the restricted 
   security
context.
  
qemu-fe would also not be entirely simple,
  
Indeed.
  
  
I do like the idea of a privileged qemu-fe performing the open and 
   passing
the fd to a restricted qemu.
   Me too.
  
  However, I get the impression that this won't
get delivered nearly as quickly as fd: passing could be.  How soon 
   do we
need image isolation for NFS?
  
Btw, this sounds similar to what Blue Swirl recommended here on v1 
   of this

   patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html
   I was thinking about simply doing fork() + setuid() at some point and
   using the FD passing structures directly. But would it bring
   advantages to have two separate executables, are they different from
   access control point of view vs. single but forked one?
  
  
   We could put together an SELinux policy that would transition
   qemu-fe to a more restricted domain (ie. no open privilege on NFS
   files) when it executes qemu-system-x86_64.
   
   Thinking about this some more, I don't really think the idea of delegating
   open of NFS files to a separate qemu-fe is very desirable. Libvirt makes 
   the
   decision on the security policy that the VM will run under, and provides
   audit records to log what resources are being assigned to the VM. From 
   that
   point onwards, we must be able to guarantee that MAC will be enforced on
   the VM, according to what we logged via the auditd system.
   
   In the case where we delegate opening of the files to qemu-fe, and allow
   its policy to open NFS files, we no longer have a guarentee that the MAC
   policy will be enforced as we originally intended. Yes, qemu-fe will very
   likely honour what we tell it and open the correct files, and yes qmeu-fe
   has lower attack surface wrt the guest than the real qemu does, but we
   still loose the guarentee of MAC enforcement from libvirt's POV.
  
  On the other hand, from a qemu POV libvirt is only one possible
  management tool. In practice, another very popular management tool for
  qemu is bash. With qemu-fe all the other tools, including direct
  invocation from the command line, would get some protection, too.
 
 That's why I said a qemu-fe like tool need not be mutually exclusive
 with exposing FD passing to a management tool like libvirt. Both
 qemu-fe and libvirt need to some mechanism to pass open FDs to the
 real QEMU.  We could needlessly invent a new communication channel
 between qemu-fe and qemu that only it can use, or we can use the
 channel we already have - QMP - to provide an interface that either
 qemu-fe or libvirt, can use to pass FDs into the real QEMU.

Or to put it another way...

It should be possible to build a 'qemu-fe' tool which does sandboxing
using soley the QEMU command line + QMP monitor. If this is not possible
then, IMHO, QMP should be considered incomplete / a failure, or may point
to other holes in QEMU's mgmt app APIs. eg perhaps it would demonstrate
that we do in fact need a libblockdriver.so for mgmt apps to query info
about disks.

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

[libvirt] [PATCH v2 34/26] snapshot: add disks to snapshot xml

[Eric Blake]

Adds an optional element to domainsnapshot, which will be used
to give user control over external snapshot filenames on input,
and specify generated filenames on output.

domainsnapshot
  ...
  disks
disk name='vda' snapshot='no'/
disk name='vdb' snapshot='internal'/
disk name='vdc' snapshot='external'
  driver type='qcow2'/
  source file='/path/to/new'/
/disk
  /disks
  domain
...
devices
  disk ...
driver name='qemu' type='raw'/
target dev='vdc'/
source file='/path/to/old'/
  /disk
/devices
  /domain
/domainsnapshot

* src/conf/domain_conf.h (_virDomainSnapshotDiskDef): New type.
(_virDomainSnapshotDef): Add new elements.
(virDomainSnapshotAlignDisks): New prototype.
* src/conf/domain_conf.c (virDomainSnapshotDiskDefClear)
(virDomainSnapshotDiskDefParseXML, disksorter)
(virDomainSnapshotAlignDisks): New functions.
(virDomainSnapshotDefParseString): Parse new fields.
(virDomainSnapshotDefFree): Clean them up.
(virDomainSnapshotDefFormat): Output them.
* src/libvirt_private.syms (domain_conf.h): Export new function.
* docs/schemas/domainsnapshot.rng (domainsnapshot, disksnapshot):
Add more xml.
* docs/formatsnapshot.html.in: Document it.
* tests/domainsnapshotxml2xmlin/disk_snapshot.xml: New test.
* tests/domainsnapshotxml2xmlout/disk_snapshot.xml: Update.
---

v2: squash in several fixes, documented over several replies to v1

 docs/formatsnapshot.html.in  |  126 +-
 docs/schemas/domainsnapshot.rng  |   52 
 src/conf/domain_conf.c   |  274 ++
 src/conf/domain_conf.h   |   22 ++-
 src/libvirt_private.syms |1 +
 tests/domainsnapshotxml2xmlin/disk_snapshot.xml  |   16 ++
 tests/domainsnapshotxml2xmlout/disk_snapshot.xml |   42 
 7 files changed, 522 insertions(+), 11 deletions(-)
 create mode 100644 tests/domainsnapshotxml2xmlin/disk_snapshot.xml

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 4158a63..953272c 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -64,10 +64,10 @@
 p
   Attributes of libvirt snapshots are stored as child elements of
   the codedomainsnapshot/code element.  At snapshot creation
-  time, only the codename/code and codedescription/code
-  elements are settable; the rest of the fields are informational
-  (and readonly) and will be filled in by libvirt when the
-  snapshot is created.
+  time, only the codename/code, codedescription/code,
+  and codedisks/code elements are settable; the rest of the
+  fields are informational (and readonly) and will be filled in by
+  libvirt when the snapshot is created.
 /p
 p
   The top-level codedomainsnapshot/code element may contain
@@ -86,6 +86,58 @@
 description is omitted when initially creating the snapshot,
 then this field will be empty.
   /dd
+  dtcodedisks/code/dt
+  ddOn input, this is an optional listing of specific
+instructions for disk snapshots; it is needed when making a
+snapshot of only a subset of the disks associated with a
+domain, or when overriding the domain defaults for how to
+snapshot each disk, or for providing specific control over
+what file name is created in an external snapshot.  On output,
+this is fully populated to show the state of each disk in the
+snapshot, including any properties that were generated by the
+hypervisor defaults.  For system checkpoints, this field is
+ignored on input and omitted on output (a system checkpoint
+implies that all disks participate in the snapshot process,
+and since the current implementation only does internal system
+checkpoints, there are no extra details to add); a future
+release may allow the use of codedisks/code with a system
+checkpoint.  This element has a list of codedisk/code
+sub-elements, describing anywhere from zero to all of the
+disks associated with the domain.  span class=sinceSince
+0.9.5/span
+dl
+  dtcodedisk/code/dt
+  ddThis sub-element describes the snapshot properties of a
+specific disk.  The attribute codename/code is
+mandatory, and must match the codelt;target
+dev='name'/gt;/code of one of
+the a href=formatdomain.html#elementsDisksdisk
+devices/a specified for the domain at the time of the
+snapshot.  The attribute codesnapshot/code is
+optional, and has the same values of the disk device
+element for a domain
+(codeno/code, codeinternal/code,
+or codeexternal/code).  Some hypervisors like ESX
+require that if specified, the snapshot mode must not
+override any snapshot mode attached to the 

Re: [libvirt] [Qemu-devel] [PATCH v4] Add support for fd: protocol

[Corey Bryant]

On 08/23/2011 11:50 AM, Kevin Wolf wrote:

Am 23.08.2011 17:26, schrieb Daniel P. Berrange:

  On Tue, Aug 23, 2011 at 11:13:34AM -0400, Corey Bryant wrote:



  On 08/22/2011 02:39 PM, Blue Swirl wrote:

  On Mon, Aug 22, 2011 at 5:42 PM, Corey Bryantcor...@linux.vnet.ibm.com   
wrote:



On 08/22/2011 01:25 PM, Anthony Liguori wrote:


On 08/22/2011 11:50 AM, Daniel P. Berrange wrote:


On Mon, Aug 22, 2011 at 11:29:12AM -0500, Anthony Liguori wrote:


I don't think it makes sense to have qemu-fe do dynamic 
labelling.
You certainly could avoid the fd passing by having qemu-fe do the
open though and just let qemu-fe run without the restricted 
security
context.


qemu-fe would also not be entirely simple,


Indeed.



I do like the idea of a privileged qemu-fe performing the open and 
passing
the fd to a restricted qemu.

  Me too.


  However, I get the impression that this won't
get delivered nearly as quickly as fd: passing could be.  How soon do 
we
need image isolation for NFS?

Btw, this sounds similar to what Blue Swirl recommended here on v1 of 
this

patch:http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg02187.html

  I was thinking about simply doing fork() + setuid() at some point and
  using the FD passing structures directly. But would it bring
  advantages to have two separate executables, are they different from
  access control point of view vs. single but forked one?



  We could put together an SELinux policy that would transition
  qemu-fe to a more restricted domain (ie. no open privilege on NFS
  files) when it executes qemu-system-x86_64.


  Thinking about this some more, I don't really think the idea of delegating
  open of NFS files to a separate qemu-fe is very desirable. Libvirt makes the
  decision on the security policy that the VM will run under, and provides
  audit records to log what resources are being assigned to the VM. From that
  point onwards, we must be able to guarantee that MAC will be enforced on
  the VM, according to what we logged via the auditd system.

  In the case where we delegate opening of the files to qemu-fe, and allow
  its policy to open NFS files, we no longer have a guarentee that the MAC
  policy will be enforced as we originally intended. Yes, qemu-fe will very
  likely honour what we tell it and open the correct files, and yes qmeu-fe
  has lower attack surface wrt the guest than the real qemu does, but we
  still loose the guarentee of MAC enforcement from libvirt's POV.

On the other hand, from a qemu POV libvirt is only one possible
management tool. In practice, another very popular management tool for
qemu is bash. With qemu-fe all the other tools, including direct
invocation from the command line, would get some protection, too.

Kevin


True, but like you said it provides just some protection.  To really be 
useful qemu-fe would need the ability to label qemu guest processes and 
image files to provide MAC isolation.


--
Regards,
Corey

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

Re: [libvirt] [RFC 06/12] Add USB companion controllers support

[Daniel P. Berrange]

On Sun, Aug 21, 2011 at 10:01:17PM +0300, Marc-André Lureau wrote:
 Companion controllers take an extra 'master' attribute to associate
 them.
 ---
  docs/formatdomain.html.in  |   20 +
  docs/schemas/domain.rng|   15 +++
  src/conf/domain_conf.c |   44 
 
  src/conf/domain_conf.h |   18 
  src/qemu/qemu_command.c|7 +++
  .../qemuxml2argv-usb-ich9-companion.args   |6 +++
  .../qemuxml2argv-usb-ich9-companion.xml|   30 +
  tests/qemuxml2argvtest.c   |5 ++
  8 files changed, 145 insertions(+), 0 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 0a383f6..5c232fa 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -1243,6 +1243,26 @@
sub-element.
  /p
  
 +p
 +  USB companion controllers have an optional
 +  sub-element codelt;mastergt;/code to specify the exact
 +  relationship of the companion to its master controller.
 +/p
 +
 +pre
 +  ...
 +  lt;devicesgt;
 +lt;controller type='usb' index='0' model='ich9-ehci1'gt;
 +  lt;address type='pci' domain='0' bus='0' slot='4' function='7'/gt;
 +lt;/controllergt;
 +lt;controller type='usb' index='1' model='ich9-uhci1'gt;
 +  lt;master bus='0' startport='0'/gt;
 +  lt;address type='pci' domain='0' bus='0' slot='4' function='0'/gt;
 +lt;/controllergt;


The 'index' attribute on controllers is used to link up to the
PCI/USB/Drive address on individual devices.

This example suggests that each companion controller is a new
bus for devices to link upto, but they don't actually work
this way. All devices will always link directly to the echi
controller, regardless of any companions. So we should not
be incrementing the 'index' for the companions.

Also, although I suggested it IIRC, we should not in fact
have a 'bus' attribute on the master element, since it
is already implied by the 'index' attribute on the controller
All we need todo is have

  master startport='0'/

to show that its a slave companion, not a new bus.

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] maint: fix comment typos

[Eric Blake]

* src/qemu/qemu_driver.c (qemuDomainSaveInternal): Fix typo.
* src/conf/domain_event.c (virDomainEventDispatchMatchCallback):
Likewise.
* daemon/libvirtd.c (daemonRunStateInit): Likewise.
* src/lxc/lxc_container.c (lxcContainerChildMountSort): Likewise.
* src/util/virterror.c (virCopyError, virRaiseErrorFull): Likewise.
* src/xenxs/xen_sxpr.c (xenParseSxprSound): Likewise.
---

Pushing under the trivial rule.

 daemon/libvirtd.c   |2 +-
 src/conf/domain_event.c |4 ++--
 src/lxc/lxc_container.c |2 +-
 src/qemu/qemu_driver.c  |2 +-
 src/util/virterror.c|4 ++--
 src/xenxs/xen_sxpr.c|2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 0530ba5..5969a82 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1131,7 +1131,7 @@ static void daemonRunStateInit(void *opaque)
 virNetServerPtr srv = opaque;

 /* Start the stateful HV drivers
- * This is delibrately done after telling the parent process
+ * This is deliberately done after telling the parent process
  * we're ready, since it can take a long time and this will
  * seriously delay OS bootup process */
 if (virStateInitialize(virNetServerIsPrivileged(srv))  0) {
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 2e0524b..3189346 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1103,11 +1103,11 @@ static int 
virDomainEventDispatchMatchCallback(virDomainEventPtr event,
 return 0;

 if (cb-dom) {
-/* Delibrately ignoring 'id' for matching, since that
+/* Deliberately ignoring 'id' for matching, since that
  * will cause problems when a domain switches between
  * running  shutoff states  ignoring 'name' since
  * Xen sometimes renames guests during migration, thus
- * leaving 'uuid' as the only truely reliable ID we can use*/
+ * leaving 'uuid' as the only truly reliable ID we can use*/

 if (memcmp(event-dom.uuid, cb-dom-uuid, VIR_UUID_BUFLEN) == 0)
 return 1;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 895ef09..e425328 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -287,7 +287,7 @@ static int lxcContainerChildMountSort(const void *a, const 
void *b)
   const char **sa = (const char**)a;
   const char **sb = (const char**)b;

-  /* Delibrately reversed args - we need to unmount deepest
+  /* Deliberately reversed args - we need to unmount deepest
  children first */
   return strcmp(*sb, *sa);
 }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 035ddb4..2c677b0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2564,7 +2564,7 @@ qemuDomainSaveInternal(struct qemud_driver *driver, 
virDomainPtr dom,
 /* Avoid throwing an error here, since it is possible
  * that with NFS we can't actually stat() the file.
  * The subsequent codepaths will still raise an error
- * if a truely fatal problem is hit */
+ * if a truly fatal problem is hit */
 is_reg = true;
 } else {
 is_reg = !!S_ISREG(sb.st_mode);
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 9a27feb..c5babb1 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -248,7 +248,7 @@ virCopyError(virErrorPtr from,
 to-int1 = from-int1;
 to-int2 = from-int2;
 /*
- * Delibrately not setting 'conn', 'dom', 'net' references
+ * Deliberately not setting 'conn', 'dom', 'net' references
  */
 return ret;
 }
@@ -708,7 +708,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
  * Save the information about the error
  */
 /*
- * Delibrately not setting conn, dom  net fields since
+ * Deliberately not setting conn, dom  net fields since
  * they're utterly unsafe
  */
 to-domain = domain;
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 7b292bc..5e278ae 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -604,7 +604,7 @@ xenParseSxprSound(virDomainDefPtr def,
  * Special compatability code for Xen with a bogus
  * sound=all in config.
  *
- * NB delibrately, don't include all possible
+ * NB deliberately, don't include all possible
  * sound models anymore, just the 2 that were
  * historically present in Xen's QEMU.
  *
-- 
1.7.4.4

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

[libvirt] [PATCH 41/26] snapshot: wire up live qemu disk snapshots

[Eric Blake]

Lots of earlier patches led up to this point - the qemu snapshot_blkdev
monitor command can now be controlled by libvirt!  Well, insofar as
SELinux doesn't prevent qemu from open(O_CREAT) on the files.  There's
still some followup work before things work with SELinux enforcing,
but this patch is big enough to post now.

There's still room for other improvements, too (for example, taking a
disk snapshot of an inactive domain, by using qemu-img for both internal
and external snapshots; wiring up delete and revert control, including
additional flags from my RFC; supporting active QED disk snapshots;
supporting per-storage-volume snapshots such as LVM or btrfs snapshots;
etc.).  But this patch is the one that proves the new XML works!

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Wire in
active disk snapshots.
(qemuDomainSnapshotDiskPrepare)
(qemuDomainSnapshotCreateDiskActive): New functions.
---
 src/qemu/qemu_driver.c |  286 +++
 1 files changed, 261 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4bb77d..e8a7985 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8700,6 +8700,218 @@ cleanup:
 return ret;
 }

+static int
+qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def)
+{
+int ret = -1;
+int i;
+bool found = false;
+bool active = virDomainObjIsActive(vm);
+struct stat st;
+
+for (i = 0; i  def-ndisks; i++) {
+virDomainSnapshotDiskDefPtr disk = def-disks[i];
+
+switch (disk-snapshot) {
+case VIR_DOMAIN_DISK_SNAPSHOT_INTERNAL:
+if (active) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(active qemu domains require external disk 
+  snapshots; disk %s requested internal),
+disk-name);
+goto cleanup;
+}
+if (!vm-def-disks[i]-driverType ||
+STRNEQ(vm-def-disks[i]-driverType, qcow2)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(internal snapshot for disk %s unsupported 
+  for storage type %s),
+disk-name,
+NULLSTR(vm-def-disks[i]-driverType));
+goto cleanup;
+}
+found = true;
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL:
+if (!disk-driverType) {
+if (!(disk-driverType = strdup(qcow2))) {
+virReportOOMError();
+goto cleanup;
+}
+} else if (STRNEQ(disk-driverType, qcow2)) {
+/* XXX We should also support QED */
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(external snapshot format for disk %s 
+  is unsupported: %s),
+disk-name, disk-driverType);
+goto cleanup;
+}
+if (stat(disk-file, st)  0) {
+if (errno != ENOENT) {
+virReportSystemError(errno,
+ _(unable to stat for disk %s: %s),
+ disk-name, disk-file);
+goto cleanup;
+}
+} else if (!S_ISBLK(st.st_mode)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(external snapshot file for disk %s already 
+  exists and is not a block device: %s),
+disk-name, disk-file);
+goto cleanup;
+}
+found = true;
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_NO:
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_DEFAULT:
+default:
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(unexpected code path));
+goto cleanup;
+}
+}
+
+if (!found) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(disk snapshots require at least one disk to be 
+  selected for snapshot));
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+return ret;
+}
+
+/* The domain is expected to be locked and active. */
+static int
+qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
+   struct qemud_driver *driver,
+   virDomainObjPtr *vmptr,
+   virDomainSnapshotObjPtr snap,
+   unsigned int flags)
+{
+virDomainObjPtr vm = *vmptr;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+bool resume = false;
+int ret = -1;
+int i;
+
+if 

Re: [libvirt] [PATCH 3/3] qemu: Deal with stucked qemu on daemon startup

[Michal Privoznik]

On 23.08.2011 14:42, Daniel P. Berrange wrote:
 On Tue, Aug 16, 2011 at 06:39:12PM +0200, Michal Privoznik wrote:
 If libvirt daemon gets restarted and there is (at least) one
 unresponsive qemu, the startup procedure hangs up. This patch creates
 one thread per vm in which we try to reconnect to monitor. Therefore,
 blocking in one thread will not affect other APIs.
 ---
  src/qemu/qemu_driver.c  |   23 +++-
  src/qemu/qemu_process.c |   87 
 ++
  2 files changed, 85 insertions(+), 25 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 421a98e..4574b6c 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -143,26 +143,15 @@ qemuAutostartDomain(void *payload, const void *name 
 ATTRIBUTE_UNUSED, void *opaq
  
  virDomainObjLock(vm);
  virResetLastError();
 -if (qemuDomainObjBeginJobWithDriver(data-driver, vm,
 -QEMU_JOB_MODIFY)  0) {
 +if (vm-autostart 
 +!virDomainObjIsActive(vm) 
 +qemuDomainObjStart(data-conn, data-driver, vm,
 +   false, false,
 +   data-driver-autoStartBypassCache)  0) {
  err = virGetLastError();
 -VIR_ERROR(_(Failed to start job on VM '%s': %s),
 +VIR_ERROR(_(Failed to autostart VM '%s': %s),
vm-def-name,
err ? err-message : _(unknown error));
 -} else {
 -if (vm-autostart 
 -!virDomainObjIsActive(vm) 
 -qemuDomainObjStart(data-conn, data-driver, vm,
 -   false, false,
 -   data-driver-autoStartBypassCache)  0) {
 -err = virGetLastError();
 -VIR_ERROR(_(Failed to autostart VM '%s': %s),
 -  vm-def-name,
 -  err ? err-message : _(unknown error));
 -}
 -
 -if (qemuDomainObjEndJob(data-driver, vm) == 0)
 -vm = NULL;
  }
 
 I'm not really seeing why this change is safe. You're removing the
 job protection from the autostart code, but AFAICT, no where else
 in the caller of this method is changed to acquire the job instead.
 
 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index 21e73a5..1daf6ae 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -820,6 +820,7 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
  {
  qemuDomainObjPrivatePtr priv = vm-privateData;
  int ret = -1;
 +qemuMonitorPtr mon = NULL;
  
  if (virSecurityManagerSetSocketLabel(driver-securityManager, vm)  0) {
  VIR_ERROR(_(Failed to set security context for monitor for %s),
 @@ -831,15 +832,29 @@ qemuConnectMonitor(struct qemud_driver *driver, 
 virDomainObjPtr vm)
   * deleted while the monitor is active */
  virDomainObjRef(vm);
  
 -priv-mon = qemuMonitorOpen(vm,
 -priv-monConfig,
 -priv-monJSON,
 -monitorCallbacks);
 +ignore_value(virTimeMs(priv-monStart));
 +virDomainObjUnlock(vm);
 +qemuDriverUnlock(driver);
 +
 +mon = qemuMonitorOpen(vm,
 +  priv-monConfig,
 +  priv-monJSON,
 +  monitorCallbacks);
 +
 +qemuDriverLock(driver);
 +virDomainObjLock(vm);
 +priv-monStart = 0;
 
 I'm also not convinced it is safe to release the driver/domain
 locks in this way. At the very *least* you need to be acquiring
 an extra reference on the virDomainObjPtr, ideally using the
 EnterMonitor/ExitMonitor methods, otherwise some other thread
 may destroy the virDomainObjPtr while it is unlocked.
I cannot use EnterMonitor because it access priv-mon which does not
exist at this time.
 
 Also what is the 'monStart' field used for ?

For gathering statistics: virsh domcontrol domain
 
  
  /* Safe to ignore value since ref count was incremented above */
 -if (priv-mon == NULL)
 +if (mon == NULL)
  ignore_value(virDomainObjUnref(vm));
  
 +if (!virDomainObjIsActive(vm)) {
 +qemuMonitorClose(mon);
 +goto error;
 +}
 +priv-mon = mon;
 +
  if (virSecurityManagerClearSocketLabel(driver-securityManager, vm)  
 0) {
  VIR_ERROR(_(Failed to clear security context for monitor for %s),
vm-def-name);
 @@ -2484,21 +2499,25 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
  struct qemuProcessReconnectData {
  virConnectPtr conn;
  struct qemud_driver *driver;
 +void *payload;
  };
  /*
   * Open an existing VM's monitor, re-detect VCPU threads
   * and re-reserve the security labels in use
   */
  static void
 -qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void 
 *opaque)
 +qemuProcessReconnect(void *opaque)
  {
 -virDomainObjPtr obj = payload;
  struct 

[libvirt] [PATCH 3/3 v2] qemu: Deal with stucked qemu on daemon startup

[Michal Privoznik]

If libvirt daemon gets restarted and there is (at least) one
unresponsive qemu, the startup procedure hangs up. This patch creates
one thread per vm in which we try to reconnect to monitor. Therefore,
blocking in one thread will not affect other APIs.
---
 src/qemu/qemu_driver.c  |   23 +
 src/qemu/qemu_process.c |  123 +++
 2 files changed, 127 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..8c2e356 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -143,16 +143,18 @@ qemuAutostartDomain(void *payload, const void *name 
ATTRIBUTE_UNUSED, void *opaq
 
 virDomainObjLock(vm);
 virResetLastError();
-if (qemuDomainObjBeginJobWithDriver(data-driver, vm,
-QEMU_JOB_MODIFY)  0) {
-err = virGetLastError();
-VIR_ERROR(_(Failed to start job on VM '%s': %s),
-  vm-def-name,
-  err ? err-message : _(unknown error));
-} else {
-if (vm-autostart 
-!virDomainObjIsActive(vm) 
-qemuDomainObjStart(data-conn, data-driver, vm,
+if (vm-autostart 
+!virDomainObjIsActive(vm)) {
+if (qemuDomainObjBeginJobWithDriver(data-driver, vm,
+QEMU_JOB_MODIFY)  0) {
+err = virGetLastError();
+VIR_ERROR(_(Failed to start job on VM '%s': %s),
+  vm-def-name,
+  err ? err-message : _(unknown error));
+goto cleanup;
+}
+
+if (qemuDomainObjStart(data-conn, data-driver, vm,
false, false,
data-driver-autoStartBypassCache)  0) {
 err = virGetLastError();
@@ -165,6 +167,7 @@ qemuAutostartDomain(void *payload, const void *name 
ATTRIBUTE_UNUSED, void *opaq
 vm = NULL;
 }
 
+cleanup:
 if (vm)
 virDomainObjUnlock(vm);
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 21e73a5..4cc8ae6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -820,6 +820,7 @@ qemuConnectMonitor(struct qemud_driver *driver, 
virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm-privateData;
 int ret = -1;
+qemuMonitorPtr mon = NULL;
 
 if (virSecurityManagerSetSocketLabel(driver-securityManager, vm)  0) {
 VIR_ERROR(_(Failed to set security context for monitor for %s),
@@ -831,15 +832,29 @@ qemuConnectMonitor(struct qemud_driver *driver, 
virDomainObjPtr vm)
  * deleted while the monitor is active */
 virDomainObjRef(vm);
 
-priv-mon = qemuMonitorOpen(vm,
-priv-monConfig,
-priv-monJSON,
-monitorCallbacks);
+ignore_value(virTimeMs(priv-monStart));
+virDomainObjUnlock(vm);
+qemuDriverUnlock(driver);
+
+mon = qemuMonitorOpen(vm,
+  priv-monConfig,
+  priv-monJSON,
+  monitorCallbacks);
+
+qemuDriverLock(driver);
+virDomainObjLock(vm);
+priv-monStart = 0;
 
 /* Safe to ignore value since ref count was incremented above */
-if (priv-mon == NULL)
+if (mon == NULL)
 ignore_value(virDomainObjUnref(vm));
 
+if (!virDomainObjIsActive(vm)) {
+qemuMonitorClose(mon);
+goto error;
+}
+priv-mon = mon;
+
 if (virSecurityManagerClearSocketLabel(driver-securityManager, vm)  0) {
 VIR_ERROR(_(Failed to clear security context for monitor for %s),
   vm-def-name);
@@ -2484,24 +2499,30 @@ qemuProcessRecoverJob(struct qemud_driver *driver,
 struct qemuProcessReconnectData {
 virConnectPtr conn;
 struct qemud_driver *driver;
+void *payload;
+struct qemuDomainJobObj oldjob;
 };
 /*
  * Open an existing VM's monitor, re-detect VCPU threads
  * and re-reserve the security labels in use
  */
 static void
-qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void 
*opaque)
+qemuProcessReconnect(void *opaque)
 {
-virDomainObjPtr obj = payload;
 struct qemuProcessReconnectData *data = opaque;
 struct qemud_driver *driver = data-driver;
+virDomainObjPtr obj = data-payload;
 qemuDomainObjPrivatePtr priv;
 virConnectPtr conn = data-conn;
 struct qemuDomainJobObj oldjob;
 
+memcpy(oldjob, data-oldjob, sizeof(oldjob));
+
+VIR_FREE(data);
+
+qemuDriverLock(driver);
 virDomainObjLock(obj);
 
-qemuDomainObjRestoreJob(obj, oldjob);
 
 VIR_DEBUG(Reconnect monitor to %p '%s', obj, obj-def-name);
 
@@ -2572,12 +2593,21 @@ qemuProcessReconnect(void *payload, const void *name 
ATTRIBUTE_UNUSED, void *opa
 if (virDomainObjUnref(obj)  0)
 virDomainObjUnlock(obj);
 
+if (qemuDomainObjEndJob(driver, obj) == 0)
+obj = NULL;
+
+qemuDriverUnlock(driver);
+
 

[libvirt] [PATCH 1/3 v2] daemon: Create priority workers pool

[Michal Privoznik]

This patch annotates APIs with low or high priority.
In low set MUST be all APIs which might eventually access monitor
(and thus block indefinitely). Other APIs may be marked as high
priority. However, some must be (e.g. domainDestroy).

For high priority calls (HPC), there is new thread pool created.
HPC tries to land in usual thread pool firstly. The condition here
is it contains at least one free worker. As soon as it doesn't,
HPCs are placed into the new pool. Therefore, only those APIs which
are guaranteed to end in reasonable small amount of time can be marked
as HPC.

The size of this HPC pool is static, because HPC are expected to end
quickly, therefore jobs assigned to this pool will be served quickly.
It can be configured in libvirtd.conf via prio_workers variable.
Default is set to 5.

To mark API with low or high priority, append priority:{low|high} to
it's comment in src/remote/remote_protocol.x. This is similar to
autogen|skipgen.
---
 daemon/libvirtd.aug   |1 +
 daemon/libvirtd.c |   10 +-
 daemon/libvirtd.conf  |6 +
 daemon/remote.c   |   26 ++
 daemon/remote.h   |2 +
 src/qemu/qemu_process.c   |2 +-
 src/remote/qemu_protocol.x|   13 +-
 src/remote/remote_protocol.x  |  544 +
 src/rpc/gendispatch.pl|   20 ++-
 src/rpc/virnetserver.c|   32 +++-
 src/rpc/virnetserver.h|6 +-
 src/rpc/virnetserverprogram.h |1 +
 src/util/threadpool.c |   38 ++-
 src/util/threadpool.h |1 +
 14 files changed, 411 insertions(+), 291 deletions(-)

diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
index 3f47ebb..ce00db5 100644
--- a/daemon/libvirtd.aug
+++ b/daemon/libvirtd.aug
@@ -57,6 +57,7 @@ module Libvirtd =
 | int_entry max_clients
 | int_entry max_requests
 | int_entry max_client_requests
+| int_entry prio_workers
 
let logging_entry = int_entry log_level
  | str_entry log_filters
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 0530ba5..3a6233d 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -134,6 +134,8 @@ struct daemonConfig {
 int max_workers;
 int max_clients;
 
+int prio_workers;
+
 int max_requests;
 int max_client_requests;
 
@@ -886,6 +888,8 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
 data-max_workers = 20;
 data-max_clients = 20;
 
+data-prio_workers = 5;
+
 data-max_requests = 20;
 data-max_client_requests = 5;
 
@@ -1042,6 +1046,8 @@ daemonConfigLoad(struct daemonConfig *data,
 GET_CONF_INT (conf, filename, max_workers);
 GET_CONF_INT (conf, filename, max_clients);
 
+GET_CONF_INT (conf, filename, prio_workers);
+
 GET_CONF_INT (conf, filename, max_requests);
 GET_CONF_INT (conf, filename, max_client_requests);
 
@@ -1433,7 +1439,9 @@ int main(int argc, char **argv) {
 config-max_clients,
 config-mdns_adv ? config-mdns_name : NULL,
 use_polkit_dbus,
-remoteClientInitHook))) {
+remoteClientInitHook,
+config-prio_workers,
+remoteGetProcPriority))) {
 ret = VIR_DAEMON_ERR_INIT;
 goto cleanup;
 }
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 95e43dd..da3983e 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -257,6 +257,12 @@
 #min_workers = 5
 #max_workers = 20
 
+
+# The number of priority workers. If all workers from above
+# pool will stuck, some calls marked as high priority
+# (notably domainDestroy) can be executed in this pool.
+#prio_workers = 5
+
 # Total global limit on concurrent RPC calls. Should be
 # at least as large as max_workers. Beyond this, RPC requests
 # will be read into memory and queued. This directly impact
diff --git a/daemon/remote.c b/daemon/remote.c
index 0f088c6..9b1433b 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -473,6 +473,32 @@ int remoteClientInitHook(virNetServerPtr srv 
ATTRIBUTE_UNUSED,
 return 0;
 }
 
+unsigned int remoteGetProcPriority(virNetMessageHeaderPtr hdr)
+{
+u_int prog = hdr-prog;
+int proc = hdr-proc;
+virNetServerProgramProc *table = NULL;
+size_t max = 0;
+
+switch (prog) {
+case REMOTE_PROGRAM:
+table = remoteProcs;
+max = remoteNProcs;
+break;
+case QEMU_PROGRAM:
+table = qemuProcs;
+max = qemuNProcs;
+break;
+}
+
+if (!table || !max)
+return 0;
+if (proc = max)
+return 0;
+
+return table[proc].priority;
+}
+
 /*- Functions. -*/
 
 static int
diff --git a/daemon/remote.h b/daemon/remote.h
index 5444e47..1969f22 100644
--- a/daemon/remote.h
+++ 

Re: [libvirt] Libvirt 0.9.4rc2 + qemu 0.15 VNC/TLS not working

[Radek Hladik]

Dne 23.8.2011 14:36, Radek Hladik napsal(a):

I am thinking whether there is not a problem with monitor setting
something after the machine starts. Libvirt does the same with password,
so maybe it does something with TLS


I tried to remove the VNC password from guest XML and TLS is working. So 
actually now the situation is like this:


* guest with password+qemu configured to use TLS = no TLS (VNC AUTH  TYPE=2)

* guest without password+qemu configured to use TLS = working TLS (VNC 
AUTH TYPE=19)


I hope it will help to make my issue more clear. I am really suspecting 
that the password setup somehow removes the TLS option from VNC.


Radek



smime.p7s
Description: Elektronický podpis S/MIME
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Curl/HTTP block device

[Radek Hladik]

Dne 16.8.2011 20:13, Daniel P. Berrange napsal(a):

On Tue, Aug 16, 2011 at 11:02:23AM -0600, Eric Blake wrote:

On 08/12/2011 04:31 AM, Radek Hladik wrote:

Hi,
I've noticed newer version of curl in last qemu changelog. It seems that
its needed for curl block device. Unfortunatelly, I was not able to find
out more. I tried to look into qemu source code and to some patches but
I am not much wiser.
As I understand it, qemu could be able to use a curl compatible URL as
block device. Probably only in readonly mode but it still would be very
usefull for i.e. ISO images.
Can anyone point me in the right direction and more importantly, is this
feature available via Libvirt?


I believe you are correct that curl block devices are not yet
supported in libvirt, but patches would be welcome.


We had previously rejected the idea of curl block devices as a crazy
idea, but since we have now accepted config of NBD, Sheepdog, and
other network based block devices, we can't justify leaving out
the curl ones. So any patches for curl block devs are welcome.

Daniel


I was briefly looking what would it need to create such patch. I've been 
checking how nbd support is done but then I found out one difference. 
NBD and sheepdog are providing one driver where CURL provides at least 5:

# ./qemu-kvm-015 -drive format=?
Supported formats: ... nbd sheepdog ... tftp ftps ftp https http
Should libvirt use all these as separate drive types or have one 
metatype i.e. curl?
On the other hand I did not find any other argument but the source URL. 
Actually it could maybe work right away with specifying URL in file 
attribute for raw device but libvirt complains about non-existing file :-)

What should disk element look like? qemu arguments are quite simple:
-drive 
file=http://rip.7bf.de/current/RIPLinux-13.5-non-X.iso,if=ide,media=cdrom,boot=on


And lastly, why do you find the idea of curl block device crazy? I think 
that its very handy solution as long as you understand the limitations 
(speed is the most important one :-) ). For example on our farm we have 
problem that if a customer wants to use some installation media or 
driver disk, we need to download it to shared storage (currently SMB 
share). Then she can use it in her guest XML definition. This way she 
just points her guest to use the device right from source URL.


Radek

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

Re: [libvirt] [PATCH 35/26] snapshot: also support disks by path

[Eric Blake]

On 08/19/2011 09:08 PM, Eric Blake wrote:

I got confused when 'virsh domblkinfo dom disk' required the
path to a disk (which can be ambiguous, since a single file
can back multiple disks), rather than the unambiguous target
device name that I was using in disk snapshots.  So, in true
developer fashion, I went for the best of both worlds - all
interfaces that operate on a disk (aka block) now accept
either the target name or the unambiguous path to the backing
file used by the disk.

@@ -11284,6 +11312,13 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr 
def,
   disk-file, disk-name);
  goto cleanup;
  }
+if (STRNEQ(disk-file, def-dom-disks[idx]-dst)) {
+VIR_FREE(disk-file);
+if (!(disk-file = strdup(def-dom-disks[idx]-dst))) {
+virReportOOMError();
+goto cleanup;
+}
+}


I got a NULL deref in testing, and traced it to the use of the wrong 
field.  Squash this in (I promise to post a clean v3 of the entire 
series once I flush out more of these little fixups).


diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index aa5f2d3..effc2a3 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -11314,9 +11314,9 @@ 
virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,

  disk-file, disk-name);
 goto cleanup;
 }
-if (STRNEQ(disk-file, def-dom-disks[idx]-dst)) {
-VIR_FREE(disk-file);
-if (!(disk-file = strdup(def-dom-disks[idx]-dst))) {
+if (STRNEQ(disk-name, def-dom-disks[idx]-dst)) {
+VIR_FREE(disk-name);
+if (!(disk-name = strdup(def-dom-disks[idx]-dst))) {
 virReportOOMError();
 goto cleanup;
 }

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

[libvirt] [PATCH] lxc: do not require 'ifconfig' or 'ipconfig' in container

[Scott Moser]

Currently, the lxc implementation invokes 'ip' and 'ifconfig' commands
inside a container using 'virRun'.  That has the side effect of requiring
those commands to be present and to function in a manner consistent with
the usage.  Some small roots (such as ttylinux) may not have 'ip' or
'ifconfig'.

This patch replaces the use of these commands with usage of
netdevice.  The result is that lxc containers do not have to implement
those commands, and lxc in libvirt is only dependent on the netdevice
interface.

I've tested this patch locally against the ubuntu libvirt version enough
to verify its generally sane.  I attempted to build upstream today, but
failed with:
  /usr/bin/ld:
../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_domain.o):
   undefined reference to symbol 'xmlXPathRegisterNs@@LIBXML2_2.4.30

Thats probably a local issue only, but I wanted to get this patch up and
see what others thought of it.  This is ubuntu bug
https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/828211 .

diff --git a/src/lxc/veth.c b/src/lxc/veth.c
index 34cb804..c24df91 100644
--- a/src/lxc/veth.c
+++ b/src/lxc/veth.c
@@ -12,8 +12,11 @@

 #include config.h

+#include linux/sockios.h
+#include net/if.h
 #include string.h
 #include stdio.h
+#include sys/ioctl.h
 #include sys/types.h
 #include sys/wait.h

@@ -186,41 +189,49 @@ int vethDelete(const char *veth)
  * @veth: name of veth device
  * @upOrDown: 0 = down, 1 = up
  *
- * Enables a veth device using the ifconfig command.  A NULL inetAddress
- * will cause it to be left off the command line.
+ * Enables a veth device using SIOCSIFFLAGS
  *
- * Returns 0 on success or -1 in case of error
+ * Returns 0 on success, -1 on failure, with errno set
  */
 int vethInterfaceUpOrDown(const char* veth, int upOrDown)
 {
-int rc;
-const char *argv[] = {ifconfig, veth, NULL, NULL};
-int cmdResult = 0;
+struct ifreq ifr;
+int fd, ret;

-if (0 == upOrDown)
-argv[2] = down;
-else
-argv[2] = up;
+if ((fd = socket(PF_PACKET, SOCK_DGRAM, 0)) == -1)
+return(-1);

-rc = virRun(argv, cmdResult);
+memset(ifr, 0, sizeof(struct ifreq));

-if (rc != 0 ||
-(WIFEXITED(cmdResult)  WEXITSTATUS(cmdResult) != 0)) {
-if (0 == upOrDown)
+if (virStrcpyStatic(ifr.ifr_name, veth) == NULL) {
+errno = EINVAL;
+return -1;
+}
+
+if ((ret = ioctl(fd, SIOCGIFFLAGS, ifr)) == 0) {
+if (upOrDown)
+ifr.ifr_flags |= IFF_UP;
+else
+ifr.ifr_flags = ~(IFF_UP | IFF_RUNNING);
+
+ret = ioctl(fd, SIOCSIFFLAGS, ifr);
+}
+
+close(fd);
+if (ret == -1)
+if (upOrDown == 0)
 /*
  * Prevent overwriting an error log which may be set
  * where an actual failure occurs.
  */
-VIR_DEBUG(Failed to disable '%s' (%d),
-  veth, WEXITSTATUS(cmdResult));
+VIR_DEBUG(Failed to disable '%s', veth);
 else
 vethError(VIR_ERR_INTERNAL_ERROR,
-  _(Failed to enable '%s' (%d)),
-  veth, WEXITSTATUS(cmdResult));
-rc = -1;
-}
+  _(Failed to enable '%s'), veth);
+else
+ret = 0;

-return rc;
+return(ret);
 }

 /**
@@ -279,17 +290,29 @@ int setMacAddr(const char* iface, const char* macaddr)
  * @iface: name of device
  * @new: new name of @iface
  *
- * Changes the name of the given device with the
- * given new name using this command:
- * ip link set @iface name @new
+ * Changes the name of the given device.
  *
- * Returns 0 on success or -1 in case of error
+ * Returns 0 on success, -1 on failure with errno set.
  */
 int setInterfaceName(const char* iface, const char* new)
 {
-const char *argv[] = {
-ip, link, set, iface, name, new, NULL
-};
+struct ifreq ifr;
+int fd = socket(PF_PACKET, SOCK_DGRAM, 0);

-return virRun(argv, NULL);
+memset(ifr, 0, sizeof(struct ifreq));
+
+if (virStrcpyStatic(ifr.ifr_name, iface) == NULL) {
+errno = EINVAL;
+return -1;
+}
+
+if (virStrcpyStatic(ifr.ifr_newname, new) == NULL) {
+errno = EINVAL;
+return -1;
+}
+
+if (ioctl(fd, SIOCSIFNAME, ifr))
+return -1;
+
+return 0;
 }

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

Re: [libvirt] [PATCH 38/26] snapshot: make it possible to audit external snapshot

[Eric Blake]

On 08/22/2011 01:51 PM, Eric Blake wrote:

Snapshots alter the set of disk image files opened by qemu, so
they must be audited.  But they don't involve a full disk definition
structure, just the new filename.  Make the next patch easier by
refactoring the audit routines to just operate on file name.


self-NACK to this patch.  I was trying to get away from needing a full 
virDomainDiskDefPtr, and succeeded in that with my first version of 
patch 41/26; but in trying to further extend things to play nicely with 
lock manager and SELinux, those clients really do need the full 
virDomainDiskDefPtr.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

[libvirt] [PATCH] esx: Refactor a repeated string in the generator

[Matthias Bolte]

---
 src/esx/esx_vi_generator.py |   24 +---
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py
index 239ec73..8a128df 100755
--- a/src/esx/esx_vi_generator.py
+++ b/src/esx/esx_vi_generator.py
@@ -1817,17 +1817,19 @@ for obj in managed_objects_by_name.values():
 
 
 
-types_typedef.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-types_typeenum.write(/* Generated by esx_vi_generator.py */\n\n)
-types_typetostring.write(/* Generated by esx_vi_generator.py */\n\n)
-types_typefromstring.write(/* Generated by esx_vi_generator.py */\n\n)
-types_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-types_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-methods_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-methods_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-methods_macro.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-helpers_header.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
-helpers_source.write(/* Generated by esx_vi_generator.py */\n\n\n\n)
+notice = /* Generated by esx_vi_generator.py */\n\n\n\n
+
+types_typedef.write(notice)
+types_typeenum.write(notice)
+types_typetostring.write(notice)
+types_typefromstring.write(notice)
+types_header.write(notice)
+types_source.write(notice)
+methods_header.write(notice)
+methods_source.write(notice)
+methods_macro.write(notice)
+helpers_header.write(notice)
+helpers_source.write(notice)
 
 
 
-- 
1.7.4.1

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

Re: [libvirt] [PATCH] esx: Refactor a repeated string in the generator

[Eric Blake]

On 08/23/2011 03:18 PM, Matthias Bolte wrote:

---
  src/esx/esx_vi_generator.py |   24 +---
  1 files changed, 13 insertions(+), 11 deletions(-)


ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCH] esx: Refactor a repeated string in the generator

[Matthias Bolte]

2011/8/23 Eric Blake ebl...@redhat.com:
 On 08/23/2011 03:18 PM, Matthias Bolte wrote:

 ---
  src/esx/esx_vi_generator.py |   24 +---
  1 files changed, 13 insertions(+), 11 deletions(-)

 ACK.

Thanks, pushed.

-- 
Matthias Bolte
http://photron.blogspot.com

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

[libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator

[Matthias Bolte]

---
 src/Makefile.am |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 8fe7120..5ba189c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -812,8 +812,9 @@ endif
 
 BUILT_SOURCES += $(ESX_DRIVER_GENERATED)
 
-$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input 
$(srcdir)/esx/esx_vi_generator.py
-   $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py
+$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \
+ $(srcdir)/esx/esx_vi_generator.py
+   $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py
 
 if WITH_ESX
 if WITH_DRIVER_MODULES
-- 
1.7.4.1

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

Re: [libvirt] [PATCH] esx: Use $(PYTHON) instead of the shebang to run the generator

[Eric Blake]

On 08/23/2011 03:42 PM, Matthias Bolte wrote:

---
  src/Makefile.am |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 8fe7120..5ba189c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -812,8 +812,9 @@ endif

  BUILT_SOURCES += $(ESX_DRIVER_GENERATED)

-$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input 
$(srcdir)/esx/esx_vi_generator.py
-   $(AM_V_GEN)srcdir=$(srcdir) $(srcdir)/esx/esx_vi_generator.py
+$(ESX_DRIVER_GENERATED): $(srcdir)/esx/esx_vi_generator.input \
+ $(srcdir)/esx/esx_vi_generator.py
+   $(AM_V_GEN)srcdir=$(srcdir) $(PYTHON) $(srcdir)/esx/esx_vi_generator.py


ACK.  [Hmm - this looks like backports of my hyperv review comments ;]

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

Re: [libvirt] [PATCH 38/26] snapshot: make it possible to audit external snapshot

[Eric Blake]

On 08/23/2011 02:44 PM, Eric Blake wrote:

On 08/22/2011 01:51 PM, Eric Blake wrote:

Snapshots alter the set of disk image files opened by qemu, so
they must be audited. But they don't involve a full disk definition
structure, just the new filename. Make the next patch easier by
refactoring the audit routines to just operate on file name.


self-NACK to this patch. I was trying to get away from needing a full
virDomainDiskDefPtr, and succeeded in that with my first version of
patch 41/26; but in trying to further extend things to play nicely with
lock manager and SELinux, those clients really do need the full
virDomainDiskDefPtr.


Actually, it turned out useful after all.  The lock manager and security 
labelling only need one disk def at a time; it is only the audit code 
that needed both old and new disk at the same time.  If I modify the 
existing disk def in place before making the label calls, then I don't 
need to write a much more difficult disk def cloning function, just to 
satisfy the one client that needs two disk defs (but really only uses 
two strings).


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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

[libvirt] [PATCHv2 41/26] snapshot: wire up live qemu disk snapshots

[Eric Blake]

Lots of earlier patches led up to this point - the qemu snapshot_blkdev
monitor command can now be controlled by libvirt!  Well, insofar as
SELinux doesn't prevent qemu from open(O_CREAT) on the files.  There's
still some followup work before things work with SELinux enforcing,
but this patch is big enough to post now.

There's still room for other improvements, too (for example, taking a
disk snapshot of an inactive domain, by using qemu-img for both internal
and external snapshots; wiring up delete and revert control, including
additional flags from my RFC; supporting active QED disk snapshots;
supporting per-storage-volume snapshots such as LVM or btrfs snapshots;
etc.).  But this patch is the one that proves the new XML works!

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Wire in
active disk snapshots.
(qemuDomainSnapshotDiskPrepare)
(qemuDomainSnapshotCreateDiskActive)
(qemuDomainSnapshotCreateSingleDiskActive): New functions.
---

v2: split per-disk handling into helper function, for easier
addition of SELinux handling in later patch

 src/qemu/qemu_driver.c |  289 +---
 1 files changed, 273 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5652e62..85c8e43 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8700,6 +8700,240 @@ cleanup:
 return ret;
 }

+static int
+qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def)
+{
+int ret = -1;
+int i;
+bool found = false;
+bool active = virDomainObjIsActive(vm);
+struct stat st;
+
+for (i = 0; i  def-ndisks; i++) {
+virDomainSnapshotDiskDefPtr disk = def-disks[i];
+
+switch (disk-snapshot) {
+case VIR_DOMAIN_DISK_SNAPSHOT_INTERNAL:
+if (active) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(active qemu domains require external disk 
+  snapshots; disk %s requested internal),
+disk-name);
+goto cleanup;
+}
+if (!vm-def-disks[i]-driverType ||
+STRNEQ(vm-def-disks[i]-driverType, qcow2)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(internal snapshot for disk %s unsupported 
+  for storage type %s),
+disk-name,
+NULLSTR(vm-def-disks[i]-driverType));
+goto cleanup;
+}
+found = true;
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL:
+if (!disk-driverType) {
+if (!(disk-driverType = strdup(qcow2))) {
+virReportOOMError();
+goto cleanup;
+}
+} else if (STRNEQ(disk-driverType, qcow2)) {
+/* XXX We should also support QED */
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(external snapshot format for disk %s 
+  is unsupported: %s),
+disk-name, disk-driverType);
+goto cleanup;
+}
+if (stat(disk-file, st)  0) {
+if (errno != ENOENT) {
+virReportSystemError(errno,
+ _(unable to stat for disk %s: %s),
+ disk-name, disk-file);
+goto cleanup;
+}
+} else if (!S_ISBLK(st.st_mode)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(external snapshot file for disk %s already 
+  exists and is not a block device: %s),
+disk-name, disk-file);
+goto cleanup;
+}
+found = true;
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_NO:
+break;
+
+case VIR_DOMAIN_DISK_SNAPSHOT_DEFAULT:
+default:
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(unexpected code path));
+goto cleanup;
+}
+}
+
+if (!found) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(disk snapshots require at least one disk to be 
+  selected for snapshot));
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+return ret;
+}
+
+/* The domain is expected to hold monitor lock.  */
+static int
+qemuDomainSnapshotCreateSingleDiskActive(virDomainObjPtr vm,
+ virDomainSnapshotDiskDefPtr snap,
+ virDomainDiskDefPtr disk)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+char *device = NULL;
+char *source = NULL;
+ 

[libvirt] [PATCH 42/26] snapshot: refactor qemu file opening

[Eric Blake]

In a SELinux or root-squashing NFS environment, libvirt has to go
through some hoops to create a new file that qemu can then open()
by name.  Snapshots are a case where we want to guarantee an empty
file that qemu can open, so refactor some existing code to make
it easier to reuse in the next patch.

* src/qemu/qemu_driver.c (qemuOpenFile): New function, pulled from...
(qemuDomainSaveInternal): ...here.
---

I debated whether to make this generic enough to also subsume some
of the virFileOpenAs shenanigans in qemuDomainSaveImageOpen; it
shouldn't be too much further work to have both places use the new
helper function, at the expense of passing at least oflags as yet
another parameter from the caller.

 src/qemu/qemu_driver.c |  228 ++--
 1 files changed, 123 insertions(+), 105 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85c8e43..c11f08e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2450,6 +2450,125 @@ qemuCompressProgramName(int compress)
 qemudSaveCompressionTypeToString(compress));
 }

+/* Internal function to properly create or open existing files, with
+ * ownership affected by qemu driver setup.  */
+static int
+qemuOpenFile(struct qemud_driver *driver, const char *path, bool *isReg,
+ bool *bypassSecurityDriver, bool bypassCache)
+{
+struct stat sb;
+bool is_reg;
+bool bypass_security = false;
+int fd = -1;
+uid_t uid = getuid();
+gid_t gid = getgid();
+int directFlag = 0;
+
+/* path might be a pre-existing block dev, in which case
+ * we need to skip the create step, and also avoid unlink
+ * in the failure case */
+if (stat(path, sb)  0) {
+/* Avoid throwing an error here, since it is possible
+ * that with NFS we can't actually stat() the file.
+ * The subsequent codepaths will still raise an error
+ * if a truly fatal problem is hit */
+is_reg = true;
+} else {
+is_reg = !!S_ISREG(sb.st_mode);
+/* If the path is regular file which exists
+ * already and dynamic_ownership is off, we don't
+ * want to change it's ownership, just open it as-is */
+if (is_reg  !driver-dynamicOwnership) {
+uid = sb.st_uid;
+gid = sb.st_gid;
+}
+}
+
+/* First try creating the file as root */
+if (bypassCache) {
+directFlag = virFileDirectFdFlag();
+if (directFlag  0) {
+qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
+_(bypass cache unsupported by this system));
+goto cleanup;
+}
+}
+if (!is_reg) {
+fd = open(path, O_WRONLY | O_TRUNC | directFlag);
+if (fd  0) {
+virReportSystemError(errno, _(unable to open %s), path);
+goto cleanup;
+}
+} else {
+int oflags = O_CREAT | O_TRUNC | O_WRONLY | directFlag;
+if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR,
+uid, gid, 0))  0) {
+/* If we failed as root, and the error was permission-denied
+   (EACCES or EPERM), assume it's on a network-connected share
+   where root access is restricted (eg, root-squashed NFS). If the
+   qemu user (driver-user) is non-root, just set a flag to
+   bypass security driver shenanigans, and retry the operation
+   after doing setuid to qemu user */
+if ((fd != -EACCES  fd != -EPERM) ||
+driver-user == getuid()) {
+virReportSystemError(-fd,
+ _(Failed to create file '%s'),
+ path);
+goto cleanup;
+}
+
+/* On Linux we can also verify the FS-type of the directory. */
+switch (virStorageFileIsSharedFS(path)) {
+case 1:
+   /* it was on a network share, so we'll continue
+* as outlined above
+*/
+   break;
+
+case -1:
+   virReportSystemError(errno,
+_(Failed to create file 
+  '%s': couldn't determine fs type),
+path);
+   goto cleanup;
+
+case 0:
+default:
+   /* local file - log the error returned by virFileOpenAs */
+   virReportSystemError(-fd,
+_(Failed to create file '%s'),
+path);
+   goto cleanup;
+}
+
+/* Retry creating the file as driver-user */
+
+if ((fd = virFileOpenAs(path, oflags,
+S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+

[libvirt] [PATCH 43/26] snapshot: use SELinux and lock manager with external snapshots

[Eric Blake]

With this, it is now possible to create external snapshots even
when SELinux is enforcing, and to protect the new file with a
lock manager.

* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Create and register
new file with proper permissions and locks.
(qemuDomainSnapshotCreateDiskActive): Update caller.
---

I ran out of time to thoroughly test this today, but wanted to at least
get it posted.  Once I test it, I'll post the v3 of my series.

Oh, and it looks like I don't have any way to conditionally unlink()
a just-created file; maybe I should revisit 42/26 to pass yet another
bool * parameter that gets updated based on whether a file was created
vs. a block device reused.

 src/qemu/qemu_driver.c |   41 ++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c11f08e..cba5929 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -87,6 +87,7 @@
 #include configmake.h
 #include threadpool.h
 #include locking/lock_manager.h
+#include locking/domain_lock.h
 #include virkeycode.h

 #define VIR_FROM_THIS VIR_FROM_QEMU
@@ -8808,7 +8809,8 @@ cleanup:

 /* The domain is expected to hold monitor lock.  */
 static int
-qemuDomainSnapshotCreateSingleDiskActive(virDomainObjPtr vm,
+qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver,
+ virDomainObjPtr vm,
  virDomainSnapshotDiskDefPtr snap,
  virDomainDiskDefPtr disk)
 {
@@ -8817,6 +8819,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virDomainObjPtr 
vm,
 char *source = NULL;
 char *driverType = NULL;
 int ret = -1;
+int fd = -1;
+char *origsrc = NULL;
+char *origdriver = NULL;

 if (snap-snapshot != VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL) {
 VIR_ERROR(unexpected code path);
@@ -8831,7 +8836,33 @@ qemuDomainSnapshotCreateSingleDiskActive(virDomainObjPtr 
vm,
 goto cleanup;
 }

-/* XXX create new file and set selinux labels */
+/* create the stub file and set selinux labels; manipulate disk in
+ * place, in a way that can be reverted on failure. */
+fd = qemuOpenFile(driver, source, NULL, NULL, false);
+if (fd  0)
+goto cleanup;
+VIR_FORCE_CLOSE(fd);
+
+origsrc = disk-src;
+disk-src = source;
+origdriver = disk-driverType;
+disk-driverType = driverType;
+
+if (virDomainLockDiskAttach(driver-lockManager, vm, disk)  0)
+goto cleanup;
+if (virSecurityManagerSetImageLabel(driver-securityManager, vm,
+disk)  0) {
+if (virDomainLockDiskDetach(driver-lockManager, vm, disk)  0)
+VIR_WARN(Unable to release lock on %s, source);
+goto cleanup;
+}
+
+disk-src = origsrc;
+origsrc = NULL;
+disk-driverType = driverType;
+origdriver = NULL;
+
+/* create the actual snapshot */
 ret = qemuMonitorDiskSnapshot(priv-mon, device, source);
 virDomainAuditDisk(vm, disk-src, source, snapshot, ret = 0);
 if (ret  0)
@@ -8851,6 +8882,10 @@ qemuDomainSnapshotCreateSingleDiskActive(virDomainObjPtr 
vm,
  * configuration changes awaiting the next boot?  */

 cleanup:
+if (origsrc) {
+disk-src = origsrc;
+disk-driverType = driverType;
+}
 VIR_FREE(device);
 VIR_FREE(source);
 VIR_FREE(driverType);
@@ -8902,7 +8937,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
 if (snap-def-disks[i].snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO)
 continue;

-ret = qemuDomainSnapshotCreateSingleDiskActive(vm,
+ret = qemuDomainSnapshotCreateSingleDiskActive(driver, vm,
snap-def-disks[i],
vm-def-disks[i]);
 if (ret  0)
-- 
1.7.4.4

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

Re: [libvirt] [RFC 09/12] RFC: Don't reserve slot 1 if a USB controller is defined there

[Wen Congyang]

At 08/22/2011 03:01 AM, Marc-André Lureau Write:
 ---
  src/qemu/qemu_command.c |   10 +-
  1 files changed, 9 insertions(+), 1 deletions(-)
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 9ea4b7c..d25f34f 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -1093,6 +1093,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
 qemuDomainPCIAddressSetPtr addrs)
  {
  int i;
  bool reservedIDE = false;
 +bool reservedUSB = false;
  bool reservedVGA = false;
  
  /* Host bridge */
 @@ -1123,13 +1124,20 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
 qemuDomainPCIAddressSetPtr addrs)
  def-controllers[i]-info.addr.pci.slot = 1;
  def-controllers[i]-info.addr.pci.function = 1;
  }
 +} else if (def-controllers[i]-type == 
 VIR_DOMAIN_CONTROLLER_TYPE_USB 
 +   def-controllers[i]-idx == 0 
 +   def-controllers[i]-info.type == 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI 
 +   def-controllers[i]-info.addr.pci.domain == 0 
 +   def-controllers[i]-info.addr.pci.bus == 0 
 +   def-controllers[i]-info.addr.pci.slot == 1) {
 +reservedUSB = true;

The first use controller's address is 0:0:1:2, you don't check the function 
value here.
I think the code should be like IDE's code:
if the type is not VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
init def-controllers[i]-info
else
check the pci address, if the address is wrong, report a error

Thanks
Wen Congyang
  }
  }
  
  /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB 
 controller)
   * hardcoded slot=1, multifunction device
   */
 -if (!reservedIDE 
 +if (!reservedIDE  !reservedUSB 
  qemuDomainPCIAddressReserveSlot(addrs, 1)  0)
  goto error;
  


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

[libvirt] [PATCH] reserve slot 1 on pci bus0

[Wen Congyang]

After supporting multi function pci device, we only reserve function 1 on slot 
1.
The user can use the other function on slot 1 in the xml config file. We should
detect this wrong usage.

---
 src/qemu/qemu_command.c |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 287ad8d..4b5734c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1072,6 +1072,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
qemuDomainPCIAddressSetPtr addrs)
 int i;
 bool reservedIDE = false;
 bool reservedVGA = false;
+int function;
 
 /* Host bridge */
 if (qemuDomainPCIAddressReserveSlot(addrs, 0)  0)
@@ -1107,9 +1108,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
qemuDomainPCIAddressSetPtr addrs)
 /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB 
controller)
  * hardcoded slot=1, multifunction device
  */
-if (!reservedIDE 
-qemuDomainPCIAddressReserveSlot(addrs, 1)  0)
-goto error;
+for (function = 0; function = QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) 
{
+if (function == 1  reservedIDE)
+/* we have reserved this pci address */
+continue;
+
+if (qemuDomainPCIAddressReserveFunction(addrs, 1, function)  0)
+goto error;
+}
 
 /* First VGA is hardcoded slot=2 */
 if (def-nvideos  0) {
-- 
1.7.1

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

Re: [libvirt] [RFC 09/12] RFC: Don't reserve slot 1 if a USB controller is defined there

[Wen Congyang]

At 08/24/2011 09:42 AM, Wen Congyang Write:
 At 08/22/2011 03:01 AM, Marc-André Lureau Write:
 ---
  src/qemu/qemu_command.c |   10 +-
  1 files changed, 9 insertions(+), 1 deletions(-)

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 9ea4b7c..d25f34f 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -1093,6 +1093,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
 qemuDomainPCIAddressSetPtr addrs)
  {
  int i;
  bool reservedIDE = false;
 +bool reservedUSB = false;
  bool reservedVGA = false;
  
  /* Host bridge */
 @@ -1123,13 +1124,20 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, 
 qemuDomainPCIAddressSetPtr addrs)
  def-controllers[i]-info.addr.pci.slot = 1;
  def-controllers[i]-info.addr.pci.function = 1;
  }
 +} else if (def-controllers[i]-type == 
 VIR_DOMAIN_CONTROLLER_TYPE_USB 
 +   def-controllers[i]-idx == 0 
 +   def-controllers[i]-info.type == 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI 
 +   def-controllers[i]-info.addr.pci.domain == 0 
 +   def-controllers[i]-info.addr.pci.bus == 0 
 +   def-controllers[i]-info.addr.pci.slot == 1) {
 +reservedUSB = true;
 
 The first use controller's address is 0:0:1:2, you don't check the function 
 value here.
 I think the code should be like IDE's code:
 if the type is not VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
 init def-controllers[i]-info
 else
 check the pci address, if the address is wrong, report a error

I read the patch 3, and find that you do not append -usb in the command line 
while using
usb controller. So all usb controllers should not use slot 1(because we may 
append
-usbdevice in the command line)

 
 Thanks
 Wen Congyang
  }
  }
  
  /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB 
 controller)
   * hardcoded slot=1, multifunction device
   */
 -if (!reservedIDE 
 +if (!reservedIDE  !reservedUSB 
  qemuDomainPCIAddressReserveSlot(addrs, 1)  0)
  goto error;
  
 
 
 --
 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] XML files

[Derek]

Hi Folks,

I'm running KVM on Debian squeeze.  I have some requirements on a new VM to 
provide customised BIOS vendor/product strings, from my experience so far this 
is accomplish with the sysinfo/smbios support.  So I have added these to the 
XML file for my VM.

I found 'virsh edit' is the recommended way to make changes to a VM 
configuration, I find the XML file is not referenced at boot, it is only saved 
to with 'virsh dumpxml'.  'virsh edit' will update both areas - hypervisor and 
XML file.

Unfortunately the changes I make for smbios and sysinfo (as below) while 
editing don't appear to be accepted when saving with 'virsh edit'.  After 
saving I receive Domain HMC XML configuration edited. but the XML file 
remains unmodified.  I wonder if they are perhaps not supported with virsh edit 
yet?

  sysinfo type='smbios'
bios
  entry name='vendor'xxx xxx/entry
  entry name='version'xxx/entry
/bios
system
  entry name='product'xxx/entry
  entry name='version'xxx/entry
  entry name='manufacturer'xxx xxx/entry
/system
  /sysinfo
  os
type arch='x86_64' machine='pc-0.12'hvm/type
boot dev='cdrom'/
smbios mode='sysinfo'/
  /os

Thanks in advance,
Derek

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