[libvirt] [sandbox][PATCH] Fix nits in virt-sandbox-service when raise ValueError
Put error msg in list when raise ValueError. This fix is for bug: [virt-sandbox-service] execute command with unsupported URI error msg is not right https://bugzilla.redhat.com/show_bug.cgi?id=967705 Signed-off-by: Wayne Sun g...@redhat.com --- bin/virt-sandbox-service |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 4496b29..1db3c09 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -878,7 +878,7 @@ def sandbox_list(args): def sandbox_reload(args): config = read_config(args.name) if isinstance(config, gi.repository.LibvirtSandbox.ConfigServiceGeneric): -raise ValueError(_(Generic Containers do not support reload)) +raise ValueError([_(Generic Containers do not support reload)]) container = SystemdContainer(uri = args.uri, config = config) container.reload(args.unitfiles) @@ -931,7 +931,7 @@ def fullpath(cmd): def execute(args): if args.uri != lxc:///: -raise ValueError(_(Can only execute commands inside of linux containers.)) +raise ValueError([_(Can only execute commands inside of linux containers.)]) myexec = [ virsh, -c, args.uri, lxc-enter-namespace ] #myexec = [ virt-sandbox-service-util, execute ] -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt supports Guest Panicked
On Mon, 2013-05-27 at 14:28 +0200, Viktor Mihajlovski wrote: On 05/27/2013 10:41 AM, Chen Fan wrote: From: chenfan chen.fan.f...@cn.fujitsu.com @@ -3185,6 +3194,9 @@ int qemuMonitorVMStatusToPausedReason(const char *status) case QEMU_MONITOR_VM_STATUS_WATCHDOG: return VIR_DOMAIN_PAUSED_WATCHDOG; +case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED: +return VIR_DOMAIN_PAUSED_GUEST_PANICKED; + /* unreachable from this point on */ case QEMU_MONITOR_VM_STATUS_LAST: ; I just have crashed the guest while libvirtd wasn't running, after a restart, virsh domstate --reason always reports paused (guest panicked) while I would expect crashed (guest panicked). Either QEMU is reporting a bogus state or the state computation is flawed somewhere... Then I will fix this bug. and remain on_crash default behavior in the XML. Thanks for your comments. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] libvirt supports Guest Panicked
From: ChenFan chen.fan.f...@cn.fujitsu.com This patch implements qemu_driver supporting guest panicked. we crashed the guest while libvirt isn't running, then restart libvirtd, we change the domain state to 'crashed'. --- examples/domain-events/events-c/event-test.c | 10 +++ include/libvirt/libvirt.h.in | 16 + src/conf/domain_conf.c | 12 ++-- src/qemu/qemu_monitor.c | 14 +++- src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 7 ++ src/qemu/qemu_process.c | 103 ++- tools/virsh-domain-monitor.c | 8 +++ 8 files changed, 169 insertions(+), 6 deletions(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index eeff50f..1b425fb 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -93,6 +93,9 @@ const char *eventToString(int event) { case VIR_DOMAIN_EVENT_PMSUSPENDED: ret = PMSuspended; break; +case VIR_DOMAIN_EVENT_CRASHED: +ret = Crashed; +break; } return ret; } @@ -209,6 +212,13 @@ static const char *eventDetailToString(int event, int detail) { break; } break; +case VIR_DOMAIN_EVENT_CRASHED: + switch ((virDomainEventCrashedDetailType) detail) { + case VIR_DOMAIN_EVENT_CRASHED_PANICKED: + ret = Panicked; + break; + } + break; } return ret; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..56c6c5c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -155,6 +155,7 @@ typedef enum { VIR_DOMAIN_RUNNING_SAVE_CANCELED = 7, /* returned from failed save process */ VIR_DOMAIN_RUNNING_WAKEUP = 8, /* returned from pmsuspended due to wakeup event */ +VIR_DOMAIN_RUNNING_CRASHED = 9, /* resumed from crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_RUNNING_LAST @@ -180,6 +181,7 @@ typedef enum { VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from snapshot */ VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */ VIR_DOMAIN_PAUSED_SNAPSHOT = 9, /* paused while creating a snapshot */ +VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked event */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_PAUSED_LAST @@ -189,6 +191,7 @@ typedef enum { typedef enum { VIR_DOMAIN_SHUTDOWN_UNKNOWN = 0,/* the reason is unknown */ VIR_DOMAIN_SHUTDOWN_USER = 1, /* shutting down on user request */ +VIR_DOMAIN_SHUTDOWN_CRASHED = 2,/* domain crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_SHUTDOWN_LAST @@ -212,6 +215,7 @@ typedef enum { typedef enum { VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */ +VIR_DOMAIN_CRASHED_PANICKED = 1, /* domain panicked */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_CRASHED_LAST @@ -3319,6 +3323,7 @@ typedef enum { VIR_DOMAIN_EVENT_STOPPED = 5, VIR_DOMAIN_EVENT_SHUTDOWN = 6, VIR_DOMAIN_EVENT_PMSUSPENDED = 7, +VIR_DOMAIN_EVENT_CRASHED = 8, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_LAST @@ -3450,6 +3455,17 @@ typedef enum { #endif } virDomainEventPMSuspendedDetailType; +/* + * Details about the 'crashed' lifecycle event + */ +typedef enum { +VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0, /* Guest was panicked */ + +#ifdef VIR_ENUM_SENTINELS +VIR_DOMAIN_EVENT_CRASHED_LAST +#endif +} virDomainEventCrashedDetailType; + /** * virConnectDomainEventCallback: * @conn: virConnect connection diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..f577fd4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -642,7 +642,8 @@ VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST, unpaused, migration canceled, save canceled, - wakeup) + wakeup, + from crashed) VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST, unknown) @@ -657,11 +658,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, watchdog, from snapshot, shutdown, - snapshot) + snapshot, + guest panicked) VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST, unknown, - user) + user, + crashed) VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, unknown, @@ -674,7 +677,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, from snapshot)
[libvirt] [PATCH 00/11] Support CHAP authentication for iscsi pool
The XMLs like (auth type='chap' login='foo' passwd='kudo'/) was introduced long ago, but it's never used for any pool backend. This implements the support first (See 6/11 for details), and based on it, using secret object for the authentication is added too. E.g. auth type='chap' username='foo' secret uuid='48dcd4a4-b25f-4fc6-8874-84797c6e3678'/ /auth Osier Yang (11): storage: Refactor the rng schema for storage pool auth storage: Support username for chap type auth storage: Add a struct for auth secret storage: Introduce XMLs to use secret object for pool auth storage: Output auth type before username storage: Support chap authentication for iscsi pool storage: Support to use secret object for iscsi chap auth storage: Update docs/formatsecret.html storage: Use the internal API to get the secret value instead storage: Improve the pool auth type parsing and formating Storage: Fix the indention of rbd test file docs/formatsecret.html.in | 10 +- docs/schemas/storagepool.rng | 51 +++--- src/conf/storage_conf.c| 179 ++--- src/conf/storage_conf.h| 28 +++- src/storage/storage_backend_iscsi.c| 114 - src/storage/storage_backend_rbd.c | 13 +- .../storagepoolxml2xmlin/pool-iscsi-auth-login.xml | 17 ++ .../pool-iscsi-auth-secret.xml | 19 +++ .../pool-iscsi-auth-username.xml | 17 ++ tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 17 -- tests/storagepoolxml2xmlin/pool-rbd.xml| 2 +- .../pool-iscsi-auth-login.xml | 20 +++ .../pool-iscsi-auth-secret.xml | 22 +++ .../pool-iscsi-auth-username.xml | 20 +++ tests/storagepoolxml2xmlout/pool-iscsi-auth.xml| 20 --- .../pool-iscsi-vendor-product.xml | 2 +- tests/storagepoolxml2xmlout/pool-rbd.xml | 2 +- tests/storagepoolxml2xmltest.c | 3 +- 18 files changed, 424 insertions(+), 132 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml delete mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-login.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-username.xml delete mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/11] Storage: Fix the indention of rbd test file
--- tests/storagepoolxml2xmlin/pool-rbd.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storagepoolxml2xmlin/pool-rbd.xml b/tests/storagepoolxml2xmlin/pool-rbd.xml index 4ee8d56..056ba6e 100644 --- a/tests/storagepoolxml2xmlin/pool-rbd.xml +++ b/tests/storagepoolxml2xmlin/pool-rbd.xml @@ -5,7 +5,7 @@ host name='localhost' port='6789'/ host name='localhost' port='6790'/ auth username='admin' type='ceph' - secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/ + secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/ /auth /source /pool -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/11] storage: Refactor the rng schema for storage pool auth
The attributes/elements for auth type chap and ceph are complete different, this separates them into groups. And add interleave for login and passwd attributes of chap type auth. --- docs/schemas/storagepool.rng | 42 ++ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 3c2158a..2595e37 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -280,28 +280,30 @@ define name='sourceinfoauth' element name='auth' - attribute name='type' -choice - valuechap/value - valueceph/value -/choice - /attribute choice -attribute name='login' - text/ -/attribute -attribute name='username' - text/ -/attribute +group + attribute name='type' +valuechap/value + /attribute + interleave +attribute name='login' + text/ +/attribute +attribute name='passwd' + text/ +/attribute + /interleave +/group +group + attribute name='type' +valueceph/value + /attribute + attribute name='username' +text/ + /attribute + ref name='sourceinfoauthsecret'/ + /group /choice - optional -attribute name='passwd' - text/ -/attribute - /optional - optional -ref name='sourceinfoauthsecret'/ - /optional /element /define -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/11] storage: Introduce XMLs to use secret object for pool auth
Using plain password is still supported for back-compat reason. Example XML: auth type='chap' username='foo' secret uuid='48dcd4a4-b25f-4fc6-8874-84797c6e3678'/ /auth * docs/schemas/storagepool.rng (Add sourceinfoauthsecret as a choice) * src/conf/storage_conf.h (union passwd and virStoragePoolAuthSecret) * src/conf/storage_conf.c (s/chap\.passwd/chap\.u\.passwd/; Add a helper virStoragePoolAuthDefFormat; Parse the secret XMLs for chap auth) * tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml: (New tests) * tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml: (Likewise) --- docs/schemas/storagepool.rng | 9 ++- src/conf/storage_conf.c| 71 +++--- src/conf/storage_conf.h| 11 +++- .../pool-iscsi-auth-secret.xml | 19 ++ .../pool-iscsi-auth-secret.xml | 22 +++ 5 files changed, 107 insertions(+), 25 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-secret.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-secret.xml diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index ba6c741..386de1b 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -295,9 +295,12 @@ text/ /attribute /choice -attribute name='passwd' - text/ -/attribute +choice + attribute name='passwd' +text/ + /attribute + ref name='sourceinfoauthsecret'/ +/choice /interleave /group group diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0047372..8f83bae 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -349,7 +349,10 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) if (source-authType == VIR_STORAGE_POOL_AUTH_CHAP) { VIR_FREE(source-auth.chap.login); -VIR_FREE(source-auth.chap.passwd); +if (source-auth.chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) +VIR_FREE(source-auth.chap.u.passwd); +else if (source-auth.chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET) +VIR_FREE(source-auth.chap.u.secret.usage); } if (source-authType == VIR_STORAGE_POOL_AUTH_CEPHX) { @@ -483,6 +486,8 @@ virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt, { char *login = NULL; char *username = NULL; +char *passwd = NULL; +int rc; login = virXPathString(string(./auth/@login), ctxt); username = virXPathString(string(./auth/@username), ctxt); @@ -507,10 +512,20 @@ virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt, else if (username) auth-login = username; -auth-passwd = virXPathString(string(./auth/@passwd), ctxt); -if (auth-passwd == NULL) { +passwd = virXPathString(string(./auth/@passwd), ctxt); +rc = virStoragePoolDefParseAuthSecret(ctxt, auth-u.secret); + +if (passwd) { +auth-type = VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD; +auth-u.passwd = passwd; +} else if (rc == 0) { +auth-type = VIR_STORAGE_POOL_AUTH_CHAP_SECRET; +} else if (rc 0) { +return -1; +} else { virReportError(VIR_ERR_XML_ERROR, %s, - _(missing auth passwd attribute)); + _(Either 'passwd' attribute or 'secret' element + should be specified)); return -1; } @@ -1048,13 +1063,30 @@ virStoragePoolDefParseFile(const char *filename) return virStoragePoolDefParse(NULL, filename); } +static void +virStoragePoolAuthDefFormat(virBufferPtr buf, +virStoragePoolAuthSecret secret) +{ +char uuid[VIR_UUID_STRING_BUFLEN]; + +virBufferAddLit(buf, secret); +if (secret.uuidUsable) { +virUUIDFormat(secret.uuid, uuid); +virBufferAsprintf(buf, uuid='%s', uuid); +} + +if (secret.usage != NULL) { +virBufferAsprintf(buf, usage='%s', secret.usage); +} +virBufferAddLit(buf, /\n); +} + static int virStoragePoolSourceFormat(virBufferPtr buf, virStoragePoolOptionsPtr options, virStoragePoolSourcePtr src) { int i, j; -char uuid[VIR_UUID_STRING_BUFLEN]; virBufferAddLit(buf, source\n); if ((options-flags VIR_STORAGE_POOL_SOURCE_HOST) src-nhost) { @@ -1129,26 +1161,23 @@ virStoragePoolSourceFormat(virBufferPtr buf, virBufferAsprintf(buf,format type='%s'/\n, format); } -if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP) -virBufferAsprintf(buf,auth type='chap' username='%s' passwd='%s'/\n, - src-auth.chap.login, - src-auth.chap.passwd); +if
[libvirt] [PATCH 07/11] storage: Support to use secret object for iscsi chap auth
Based on the plain password chap auth support, this gets the secret value (password) with the secret driver methods, and apply it for the iscsiadm update command. --- src/storage/storage_backend_iscsi.c | 56 + 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 6a17b5a..516025a 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -35,6 +35,8 @@ #include unistd.h #include sys/stat.h +#include datatypes.h +#include driver.h #include virerror.h #include storage_backend_scsi.h #include storage_backend_iscsi.h @@ -42,6 +44,7 @@ #include virlog.h #include virfile.h #include vircommand.h +#include virobject.h #include virrandom.h #include virstring.h @@ -746,10 +749,17 @@ cleanup: } static int -virStorageBackendISCSISetAuth(virStoragePoolDefPtr def, +virStorageBackendISCSISetAuth(virConnectPtr conn, + virStoragePoolDefPtr def, const char *portal, const char *target) { +virSecretPtr secret = NULL; +unsigned char *secret_value = NULL; +const char *passwd = NULL; +virStoragePoolAuthChap chap = def-source.auth.chap; +int ret = -1; + if (def-source.authType == VIR_STORAGE_POOL_AUTH_NONE) return 0; @@ -759,6 +769,35 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def, return -1; } +if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET) { +if (chap.u.secret.uuidUsable) +secret = virSecretLookupByUUID(conn, chap.u.secret.uuid); +else +secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI, +chap.u.secret.usage); + +if (secret) { +size_t secret_size; +secret_value = conn-secretDriver-secretGetValue(secret, secret_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); +if (!secret_value) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(could not get the value of the secret + for username %s), chap.login); +goto cleanup; +} +} else { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(username '%s' specified but secret not found), + chap.login); +goto cleanup; +} + +passwd = (const char *)secret_value; +} else if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) { +passwd = chap.u.passwd; +} + if (virStorageBackendISCSINodeUpdate(portal, target, node.session.auth.authmethod, @@ -770,14 +809,18 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def, virStorageBackendISCSINodeUpdate(portal, target, node.session.auth.password, - def-source.auth.chap.u.passwd) 0) -return -1; + passwd) 0) +goto cleanup; -return 0; +ret = 0; +cleanup: +virObjectUnref(secret); +VIR_FREE(secret_value); +return ret; } static int -virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, +virStorageBackendISCSIStartPool(virConnectPtr conn, virStoragePoolObjPtr pool) { char *portal = NULL; @@ -817,7 +860,8 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, NULL, NULL) 0) goto cleanup; -if (virStorageBackendISCSISetAuth(pool-def, +if (virStorageBackendISCSISetAuth(conn, + pool-def, portal, pool-def-source.devices[0].path) 0) goto cleanup; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/11] storage: Update docs/formatsecret.html
To mention that the secret type iscsi and ceph can be used to storage pool too. --- docs/formatsecret.html.in | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 50c9533..3e306b5 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -64,8 +64,9 @@ a single codename/code element that specifies a usage name for the secret. The Ceph secret can then be used by UUID or by this usage name via the codelt;authgt;/code element of - a a href=formatdomain.html#elementsDisksdisk - device/a. span class=sinceSince 0.9.7/span. + a a href=formatdomain.html#elementsDisksdisk device/a or + a a href=formatstorage.htmlstorage pool (rbd)/a. + span class=sinceSince 0.9.7/span. /p h3Usage type iscsi/h3 @@ -76,8 +77,9 @@ a single codetarget/code element that specifies a usage name for the secret. The iSCSI secret can then be used by UUID or by this usage name via the codelt;authgt;/code element of - a a href=formatdomain.html#elementsDisksdisk - device/a. span class=sinceSince 1.0.4/span. + a a href=formatdomain.html#elementsDisksdisk device/a or + a a href=formatstorage.htmlstorage pool (iscsi)/a. + span class=sinceSince 1.0.4/span. /p h2a name=exampleExample/a/h2 -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/11] storage: Output auth type before username
As a habit, type attribute is printed first. --- src/conf/storage_conf.c | 2 +- tests/storagepoolxml2xmlout/pool-rbd.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8f83bae..a876332 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1175,7 +1175,7 @@ virStoragePoolSourceFormat(virBufferPtr buf, } if (src-authType == VIR_STORAGE_POOL_AUTH_CEPHX) { -virBufferAsprintf(buf,auth username='%s' type='ceph'\n, +virBufferAsprintf(buf,auth type='ceph' username='%s'\n, src-auth.cephx.username); virStoragePoolAuthDefFormat(buf, src-auth.cephx.secret); virBufferAddLit(buf,/auth\n); diff --git a/tests/storagepoolxml2xmlout/pool-rbd.xml b/tests/storagepoolxml2xmlout/pool-rbd.xml index 309a6d9..4fe2fce 100644 --- a/tests/storagepoolxml2xmlout/pool-rbd.xml +++ b/tests/storagepoolxml2xmlout/pool-rbd.xml @@ -8,7 +8,7 @@ namerbd/name host name='localhost' port='6789'/ host name='localhost' port='6790'/ -auth username='admin' type='ceph' +auth type='ceph' username='admin' secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/ /auth /source -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/11] storage: Add a struct for auth secret
This also abstracts the code for parsing the XMLs to use the secret object as a helper. --- src/conf/storage_conf.c | 68 - src/conf/storage_conf.h | 14 ++ 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c0bf084..0047372 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -439,6 +439,43 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, } } +static int +virStoragePoolDefParseAuthSecret(xmlXPathContextPtr ctxt, + virStoragePoolAuthSecretPtr secret) + +{ +char *uuid = NULL; +int ret = -1; + +uuid = virXPathString(string(./auth/secret/@uuid), ctxt); +secret-usage = virXPathString(string(./auth/secret/@usage), ctxt); +if (uuid == NULL secret-usage == NULL) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(missing auth secret uuid or usage attribute)); +return -1; +} + +if (uuid != NULL) { +if (secret-usage != NULL) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(either auth secret uuid or usage expected)); +goto cleanup; +} +if (virUUIDParse(uuid, secret-uuid) 0) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(invalid auth secret uuid)); +goto cleanup; +} +secret-uuidUsable = true; +} else { +secret-uuidUsable = false; +} + +ret = 0; +cleanup: +VIR_FREE(uuid); +return ret; +} static int virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt, @@ -484,9 +521,6 @@ static int virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt, virStoragePoolAuthCephxPtr auth) { -char *uuid = NULL; -int ret = -1; - auth-username = virXPathString(string(./auth/@username), ctxt); if (auth-username == NULL) { virReportError(VIR_ERR_XML_ERROR, %s, @@ -494,34 +528,10 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt, return -1; } -uuid = virXPathString(string(./auth/secret/@uuid), ctxt); -auth-secret.usage = virXPathString(string(./auth/secret/@usage), ctxt); -if (uuid == NULL auth-secret.usage == NULL) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(missing auth secret uuid or usage attribute)); +if (virStoragePoolDefParseAuthSecret(ctxt, auth-secret) 0) return -1; -} - -if (uuid != NULL) { -if (auth-secret.usage != NULL) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(either auth secret uuid or usage expected)); -goto cleanup; -} -if (virUUIDParse(uuid, auth-secret.uuid) 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(invalid auth secret uuid)); -goto cleanup; -} -auth-secret.uuidUsable = true; -} else { -auth-secret.uuidUsable = false; -} -ret = 0; -cleanup: -VIR_FREE(uuid); -return ret; +return 0; } static int diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 8e739ff..aff1393 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -145,6 +145,14 @@ enum virStoragePoolAuthType { VIR_STORAGE_POOL_AUTH_CEPHX, }; +typedef struct _virStoragePoolAuthSecret virStoragePoolAuthSecret; +typedef virStoragePoolAuthSecret *virStoragePoolAuthSecretPtr; +struct _virStoragePoolAuthSecret { +unsigned char uuid[VIR_UUID_BUFLEN]; +char *usage; +bool uuidUsable; +}; + typedef struct _virStoragePoolAuthChap virStoragePoolAuthChap; typedef virStoragePoolAuthChap *virStoragePoolAuthChapPtr; struct _virStoragePoolAuthChap { @@ -156,11 +164,7 @@ typedef struct _virStoragePoolAuthCephx virStoragePoolAuthCephx; typedef virStoragePoolAuthCephx *virStoragePoolAuthCephxPtr; struct _virStoragePoolAuthCephx { char *username; -struct { -unsigned char uuid[VIR_UUID_BUFLEN]; -char *usage; -bool uuidUsable; -} secret; +virStoragePoolAuthSecret secret; }; /* -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/11] storage: Support username for chap type auth
To be consistent with what we use in disk auth, and ceph type storage auth, this supports username. login is still supported for back-compat reason. --- docs/schemas/storagepool.rng | 12 +++--- src/conf/storage_conf.c| 27 ++ .../storagepoolxml2xmlin/pool-iscsi-auth-login.xml | 17 ++ .../pool-iscsi-auth-username.xml | 17 ++ tests/storagepoolxml2xmlin/pool-iscsi-auth.xml | 17 -- .../pool-iscsi-auth-login.xml | 20 .../pool-iscsi-auth-username.xml | 20 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml| 20 .../pool-iscsi-vendor-product.xml | 2 +- tests/storagepoolxml2xmltest.c | 3 ++- 10 files changed, 109 insertions(+), 46 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml delete mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-login.xml create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth-username.xml delete mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 2595e37..ba6c741 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -286,9 +286,15 @@ valuechap/value /attribute interleave -attribute name='login' - text/ -/attribute +choice + !-- Legacy, but still supported to keep back-compat -- + attribute name='login' +text/ + /attribute + attribute name='username' +text/ + /attribute +/choice attribute name='passwd' text/ /attribute diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ed9effd..c0bf084 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -444,13 +444,32 @@ static int virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt, virStoragePoolAuthChapPtr auth) { -auth-login = virXPathString(string(./auth/@login), ctxt); -if (auth-login == NULL) { +char *login = NULL; +char *username = NULL; + +login = virXPathString(string(./auth/@login), ctxt); +username = virXPathString(string(./auth/@username), ctxt); + +if (!login !username) { virReportError(VIR_ERR_XML_ERROR, %s, - _(missing auth login attribute)); + _(missing auth login or username attribute)); return -1; } +if (login username) { +virReportError(VIR_ERR_XML_ERROR, %s, + _('login' is legacy name of 'username', they + are exclusive)); +VIR_FREE(login); +VIR_FREE(username); +return -1; +} + +if (login) +auth-login = login; +else if (username) +auth-login = username; + auth-passwd = virXPathString(string(./auth/@passwd), ctxt); if (auth-passwd == NULL) { virReportError(VIR_ERR_XML_ERROR, %s, @@ -1101,7 +1120,7 @@ virStoragePoolSourceFormat(virBufferPtr buf, } if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP) -virBufferAsprintf(buf,auth type='chap' login='%s' passwd='%s'/\n, +virBufferAsprintf(buf,auth type='chap' username='%s' passwd='%s'/\n, src-auth.chap.login, src-auth.chap.passwd); diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml b/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml new file mode 100644 index 000..f7d4d52 --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-iscsi-auth-login.xml @@ -0,0 +1,17 @@ +pool type='iscsi' + namevirtimages/name + uuide9392370-2917-565e-692b-d057f46512d6/uuid + source +host name=iscsi.example.com/ +device path=demo-target/ +auth type='chap' login='foobar' passwd='frobbar'/ + /source + target +path/dev/disk/by-path/path +permissions + mode0700/mode + owner0/owner + group0/group +/permissions + /target +/pool diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml b/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml new file mode 100644 index 000..b8e4d37 --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-iscsi-auth-username.xml @@ -0,0 +1,17 @@ +pool type='iscsi' + namevirtimages/name + uuide9392370-2917-565e-692b-d057f46512d6/uuid + source +host name=iscsi.example.com/ +device path=demo-target/ +auth type='chap' username='foobar' passwd='frobbar'/ + /source + target +path/dev/disk/by-path/path +permissions +
[libvirt] [PATCH 06/11] storage: Support chap authentication for iscsi pool
Though the XML for chap authentication with plain password was introduced long ago, the function was never implemented. This patch completes it. There are two types of CHAP configurations supported for iSCSI authentication: * Initiator Authentication Forward, one-way; The initiator is authenticated by the target. * Target Authentication Reverse, Bi-directional, mutual, two-way; The target is authenticated by the initiator; This method also requires Initiator Authentication This only supports the Initiator Authentication. (I don't have any enterprise iSCSI env for testing, only have a iSCSI target setup with tgtd, which doesn't support Target Authentication). Discovery authentication is not supported by tgt yet too. So this only setup the session authentication by executing 3 iscsiadm commands, E.g: % iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \ node.session.auth.authmethod -v CHAP --op update % iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \ node.session.auth.username -v Jim --op update % iscsiadm -m node --target iqn.2013-05.test:iscsi.foo --name \ node.session.auth.password -v Jimsecret --op update --- src/storage/storage_backend_iscsi.c | 68 + 1 file changed, 68 insertions(+) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index ad38ab2..6a17b5a 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -713,6 +713,68 @@ virStorageBackendISCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +static int +virStorageBackendISCSINodeUpdate(const char *portal, + const char *target, + const char *name, + const char *value) +{ + virCommandPtr cmd = NULL; + int status; + int ret = -1; + + cmd = virCommandNewArgList(ISCSIADM, +--mode, node, +--portal, portal, +--target, target, +--op, update, +--name, name, +--value, value, +NULL); + +if (virCommandRun(cmd, status) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to update '%s' of node mode for target '%s'), + name, target); +goto cleanup; +} + +ret = 0; +cleanup: +virCommandFree(cmd); +return ret; +} + +static int +virStorageBackendISCSISetAuth(virStoragePoolDefPtr def, + const char *portal, + const char *target) +{ +if (def-source.authType == VIR_STORAGE_POOL_AUTH_NONE) +return 0; + +if (def-source.authType != VIR_STORAGE_POOL_AUTH_CHAP) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(iscsi pool only supports 'chap' auth type)); +return -1; +} + +if (virStorageBackendISCSINodeUpdate(portal, + target, + node.session.auth.authmethod, + CHAP) 0 || +virStorageBackendISCSINodeUpdate(portal, + target, + node.session.auth.username, + def-source.auth.chap.login) 0 || +virStorageBackendISCSINodeUpdate(portal, + target, + node.session.auth.password, + def-source.auth.chap.u.passwd) 0) +return -1; + +return 0; +} static int virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -745,6 +807,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, if ((session = virStorageBackendISCSISession(pool, 1)) == NULL) { if ((portal = virStorageBackendISCSIPortal(pool-def-source)) == NULL) goto cleanup; + /* * iscsiadm doesn't let you login to a target, unless you've * first issued a 'sendtargets' command to the portal :-( @@ -754,6 +817,11 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, NULL, NULL) 0) goto cleanup; +if (virStorageBackendISCSISetAuth(pool-def, + portal, + pool-def-source.devices[0].path) 0) +goto cleanup; + if (virStorageBackendISCSIConnection(portal, pool-def-source.initiator.iqn, pool-def-source.devices[0].path, -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com
[libvirt] [PATCH 09/11] storage: Use the internal API to get the secret value instead
Without the flag VIR_SECRET_GET_VALUE_INTERNAL_CALL, there is no way to get the value of private secret. And error out if the secret value is not found. --- src/storage/storage_backend_rbd.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 953a8ee..d66d3f9 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -23,6 +23,7 @@ #include config.h +#include datatypes.h #include virerror.h #include storage_backend_rbd.h #include storage_conf.h @@ -88,7 +89,17 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr *ptr, goto cleanup; } -secret_value = virSecretGetValue(secret, secret_value_size, 0); +secret_value = conn-secretDriver-secretGetValue(secret, secret_value_size, 0, + VIR_SECRET_GET_VALUE_INTERNAL_CALL); + +if (!secret_value) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(could not get the value of the secret + for username %s), + pool-def-source.auth.cephx.username); +goto cleanup; +} + base64_encode_alloc((char *)secret_value, secret_value_size, rados_key); memset(secret_value, 0, secret_value_size); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/11] storage: Improve the pool auth type parsing and formating
Use the helpers Type{From,To}String instead. --- src/conf/storage_conf.c | 17 ++--- src/conf/storage_conf.h | 3 +++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a876332..bc8ca33 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -94,6 +94,10 @@ VIR_ENUM_IMPL(virStoragePoolSourceAdapterType, VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST, default, scsi_host, fc_host) +VIR_ENUM_IMPL(virStoragePoolAuthType, + VIR_STORAGE_POOL_AUTH_LAST, + none, chap, ceph) + typedef const char *(*virStorageVolFormatToString)(int format); typedef int (*virStorageVolFormatFromString)(const char *format); @@ -709,11 +713,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, if (authType == NULL) { source-authType = VIR_STORAGE_POOL_AUTH_NONE; } else { -if (STREQ(authType, chap)) { -source-authType = VIR_STORAGE_POOL_AUTH_CHAP; -} else if (STREQ(authType, ceph)) { -source-authType = VIR_STORAGE_POOL_AUTH_CEPHX; -} else { +if ((source-authType = + virStoragePoolAuthTypeTypeFromString(authType)) 0) { virReportError(VIR_ERR_XML_ERROR, _(unknown auth type '%s'), authType); @@ -1162,7 +1163,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, } if (src-authType == VIR_STORAGE_POOL_AUTH_CHAP) { -virBufferAsprintf(buf,auth type='chap' username='%s', +virBufferAsprintf(buf,auth type='%s' username='%s', + virStoragePoolAuthTypeTypeToString(src-authType), src-auth.chap.login); if (src-auth.chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) { virBufferAsprintf(buf, passwd='%s'/\n, @@ -1175,7 +1177,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, } if (src-authType == VIR_STORAGE_POOL_AUTH_CEPHX) { -virBufferAsprintf(buf,auth type='ceph' username='%s'\n, +virBufferAsprintf(buf,auth type='%s' username='%s'\n, + virStoragePoolAuthTypeTypeToString(src-authType), src-auth.cephx.username); virStoragePoolAuthDefFormat(buf, src-auth.cephx.secret); virBufferAddLit(buf,/auth\n); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index b52cdc4..06cd5c0 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -143,7 +143,10 @@ enum virStoragePoolAuthType { VIR_STORAGE_POOL_AUTH_NONE, VIR_STORAGE_POOL_AUTH_CHAP, VIR_STORAGE_POOL_AUTH_CEPHX, + +VIR_STORAGE_POOL_AUTH_LAST, }; +VIR_ENUM_DECL(virStoragePoolAuthType) typedef struct _virStoragePoolAuthSecret virStoragePoolAuthSecret; typedef virStoragePoolAuthSecret *virStoragePoolAuthSecretPtr; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.
When src is NULL, vir_strdup will return 0 directly. This patch will set dest to NULL before vir_strdup return. Signed-off-by: yangdongsheng yangds.f...@cn.fujitsu.com --- src/util/virstring.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index b244e6c..25b5d81 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -540,8 +540,10 @@ virStrdup(char **dest, const char *funcname, size_t linenr) { -if (!src) +if (!src) { +*dest = NULL; return 0; +} if (!(*dest = strdup(src))) { if (report) virReportOOMErrorFull(domcode, filename, funcname, linenr); -- 1.7.10.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.0.6
On 05/27/2013 03:44 PM, Daniel Veillard wrote: As planned, I just tagged the rc1 release in git and pushed tarball and rpms to the FTP repository: ftp://libvirt.org/ So focuse should be given to bug fixes this week, and if everything goes well I will make the release on Monday 3, a week from now. I gave a bit of testing to the rc1 rpms, seems to work okay for KVM on Fedora but further testing would be a good idea, on other distro/platforms and also other hypervisor, especially with VirtualBox as the driver was mdified to now rely on the daemon for support of that hypervisor. thanks in advance, Daniel Basic tests on s390 show no problems so far. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox][PATCH] Fix nits in virt-sandbox-service when raise ValueError
On 05/28/2013 02:01 PM, Wayne Sun wrote: Put error msg in list when raise ValueError. This fix is for bug: [virt-sandbox-service] execute command with unsupported URI error msg is not right https://bugzilla.redhat.com/show_bug.cgi?id=967705 Signed-off-by: Wayne Sung...@redhat.com --- bin/virt-sandbox-service |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 4496b29..1db3c09 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -878,7 +878,7 @@ def sandbox_list(args): def sandbox_reload(args): config = read_config(args.name) if isinstance(config, gi.repository.LibvirtSandbox.ConfigServiceGeneric): -raise ValueError(_(Generic Containers do not support reload)) +raise ValueError([_(Generic Containers do not support reload)]) container = SystemdContainer(uri = args.uri, config = config) container.reload(args.unitfiles) @@ -931,7 +931,7 @@ def fullpath(cmd): def execute(args): if args.uri != lxc:///: -raise ValueError(_(Can only execute commands inside of linux containers.)) +raise ValueError([_(Can only execute commands inside of linux containers.)]) myexec = [ virsh, -c, args.uri, lxc-enter-namespace ] #myexec = [ virt-sandbox-service-util, execute ] ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/3] virsh: supports guest panicked
Add 'virsh domstate --reason' command is effective when domain crashed. --- tools/virsh-domain-monitor.c | 8 1 file changed, 8 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 5ed89d1..4bddefe 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -192,6 +192,8 @@ vshDomainStateReasonToString(int state, int reason) return N_(save canceled); case VIR_DOMAIN_RUNNING_WAKEUP: return N_(event wakeup); +case VIR_DOMAIN_RUNNING_CRASHED: +return N_(from crashed); case VIR_DOMAIN_RUNNING_UNKNOWN: case VIR_DOMAIN_RUNNING_LAST: ; @@ -226,6 +228,8 @@ vshDomainStateReasonToString(int state, int reason) return N_(shutting down); case VIR_DOMAIN_PAUSED_SNAPSHOT: return N_(creating snapshot); +case VIR_DOMAIN_PAUSED_GUEST_PANICKED: +return N_(guest panicked); case VIR_DOMAIN_PAUSED_UNKNOWN: case VIR_DOMAIN_PAUSED_LAST: ; @@ -236,6 +240,8 @@ vshDomainStateReasonToString(int state, int reason) switch ((virDomainShutdownReason) reason) { case VIR_DOMAIN_SHUTDOWN_USER: return N_(user); +case VIR_DOMAIN_SHUTDOWN_CRASHED: +return N_(crashed); case VIR_DOMAIN_SHUTDOWN_UNKNOWN: case VIR_DOMAIN_SHUTDOWN_LAST: ; @@ -266,6 +272,8 @@ vshDomainStateReasonToString(int state, int reason) case VIR_DOMAIN_CRASHED: switch ((virDomainCrashedReason) reason) { +case VIR_DOMAIN_CRASHED_PANICKED: +return N_(panicked); case VIR_DOMAIN_CRASHED_UNKNOWN: case VIR_DOMAIN_CRASHED_LAST: ; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/3] qemu: Supports guest panicked
Add monitor callback API domainGUESTPanicked, that implements the 'on_crash' behavior in the XML when domain crashed. --- src/qemu/qemu_monitor.c | 14 +- src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 7 +++ src/qemu/qemu_process.c | 103 ++- 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4e35f79..e0cd62c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -113,7 +113,7 @@ VIR_ENUM_IMPL(qemuMonitorVMStatus, QEMU_MONITOR_VM_STATUS_LAST, debug, inmigrate, internal-error, io-error, paused, postmigrate, prelaunch, finish-migrate, restore-vm, - running, save-vm, shutdown, watchdog) + running, save-vm, shutdown, watchdog, guest-panicked) typedef enum { QEMU_MONITOR_BLOCK_IO_STATUS_OK, @@ -1032,6 +1032,15 @@ int qemuMonitorEmitResume(qemuMonitorPtr mon) } +int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon) +{ +int ret = -1; +VIR_DEBUG(mon=%p, mon); +QEMU_MONITOR_CALLBACK(mon, ret, domainGUESTPanicked, mon-vm); +return ret; +} + + int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset) { int ret = -1; @@ -3185,6 +3194,9 @@ int qemuMonitorVMStatusToPausedReason(const char *status) case QEMU_MONITOR_VM_STATUS_WATCHDOG: return VIR_DOMAIN_PAUSED_WATCHDOG; +case QEMU_MONITOR_VM_STATUS_GUEST_PANICKED: +return VIR_DOMAIN_PAUSED_GUEST_PANICKED; + /* unreachable from this point on */ case QEMU_MONITOR_VM_STATUS_LAST: ; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index a607712..543050c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -140,6 +140,8 @@ struct _qemuMonitorCallbacks { unsigned long long actual); int (*domainPMSuspendDisk)(qemuMonitorPtr mon, virDomainObjPtr vm); +int (*domainGUESTPanicked)(qemuMonitorPtr mon, + virDomainObjPtr vm); }; char *qemuMonitorEscapeArg(const char *in); @@ -220,6 +222,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, unsigned long long actual); int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon); +int qemuMonitorEmitGUESTPanicked(qemuMonitorPtr mon); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); @@ -239,6 +243,7 @@ typedef enum { QEMU_MONITOR_VM_STATUS_SAVE_VM, QEMU_MONITOR_VM_STATUS_SHUTDOWN, QEMU_MONITOR_VM_STATUS_WATCHDOG, +QEMU_MONITOR_VM_STATUS_GUEST_PANICKED, QEMU_MONITOR_VM_STATUS_LAST } qemuMonitorVMStatus; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2b73884..b6efa52 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -74,6 +74,7 @@ static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONVal static void qemuMonitorJSONHandleBlockJobReady(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandlePMSuspendDisk(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -87,6 +88,7 @@ static qemuEventHandler eventHandlers[] = { { BLOCK_JOB_COMPLETED, qemuMonitorJSONHandleBlockJobCompleted, }, { BLOCK_JOB_READY, qemuMonitorJSONHandleBlockJobReady, }, { DEVICE_TRAY_MOVED, qemuMonitorJSONHandleTrayChange, }, +{ GUEST_PANICKED, qemuMonitorJSONHandleGUESTPanicked, }, { POWERDOWN, qemuMonitorJSONHandlePowerdown, }, { RESET, qemuMonitorJSONHandleReset, }, { RESUME, qemuMonitorJSONHandleResume, }, @@ -593,6 +595,11 @@ static void qemuMonitorJSONHandleResume(qemuMonitorPtr mon, virJSONValuePtr data qemuMonitorEmitResume(mon); } +static void qemuMonitorJSONHandleGUESTPanicked(qemuMonitorPtr mon, virJSONValuePtr data ATTRIBUTE_UNUSED) +{ +qemuMonitorEmitGUESTPanicked(mon); +} + static void qemuMonitorJSONHandleRTCChange(qemuMonitorPtr mon, virJSONValuePtr data) { long long offset = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d4fd4fb..29c8c4b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -548,6 +548,7 @@ qemuProcessFakeReboot(void *opaque) qemuDomainObjPrivatePtr priv = vm-privateData; virDomainEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED; int ret = -1; VIR_DEBUG(vm=%p, vm); virObjectLock(vm); @@ -573,8 +574,11 @@ qemuProcessFakeReboot(void *opaque) goto endjob; } +if
[libvirt] [PATCH v3 0/3] libvirt supports Guest panicked
This patchs implement the 'on_crash' behavior in the XML example XML: on_crashdestroy/on_crash Changes: v2-v3: 1. split into 3 patches v1-v2: 1. fix the incorrect domain state: paused - crashed, when crash the guest while libvirt isn't running, then restart libvirtd. Chen Fan (3): libvirt: Define domain crash event types qemu: Supports guest panicked virsh: supports guest panicked examples/domain-events/events-c/event-test.c | 10 +++ include/libvirt/libvirt.h.in | 16 + src/conf/domain_conf.c | 12 ++-- src/qemu/qemu_monitor.c | 14 +++- src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 7 ++ src/qemu/qemu_process.c | 103 ++- tools/virsh-domain-monitor.c | 8 +++ 8 files changed, 169 insertions(+), 6 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/3] libvirt: Define domain crash event types
This patch introduces domain crashed types and crashed reasons which will be used while guest panicked. --- examples/domain-events/events-c/event-test.c | 10 ++ include/libvirt/libvirt.h.in | 16 src/conf/domain_conf.c | 12 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index eeff50f..1b425fb 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -93,6 +93,9 @@ const char *eventToString(int event) { case VIR_DOMAIN_EVENT_PMSUSPENDED: ret = PMSuspended; break; +case VIR_DOMAIN_EVENT_CRASHED: +ret = Crashed; +break; } return ret; } @@ -209,6 +212,13 @@ static const char *eventDetailToString(int event, int detail) { break; } break; +case VIR_DOMAIN_EVENT_CRASHED: + switch ((virDomainEventCrashedDetailType) detail) { + case VIR_DOMAIN_EVENT_CRASHED_PANICKED: + ret = Panicked; + break; + } + break; } return ret; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..56c6c5c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -155,6 +155,7 @@ typedef enum { VIR_DOMAIN_RUNNING_SAVE_CANCELED = 7, /* returned from failed save process */ VIR_DOMAIN_RUNNING_WAKEUP = 8, /* returned from pmsuspended due to wakeup event */ +VIR_DOMAIN_RUNNING_CRASHED = 9, /* resumed from crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_RUNNING_LAST @@ -180,6 +181,7 @@ typedef enum { VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from snapshot */ VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */ VIR_DOMAIN_PAUSED_SNAPSHOT = 9, /* paused while creating a snapshot */ +VIR_DOMAIN_PAUSED_GUEST_PANICKED = 10, /* paused due to a guest panicked event */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_PAUSED_LAST @@ -189,6 +191,7 @@ typedef enum { typedef enum { VIR_DOMAIN_SHUTDOWN_UNKNOWN = 0,/* the reason is unknown */ VIR_DOMAIN_SHUTDOWN_USER = 1, /* shutting down on user request */ +VIR_DOMAIN_SHUTDOWN_CRASHED = 2,/* domain crashed */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_SHUTDOWN_LAST @@ -212,6 +215,7 @@ typedef enum { typedef enum { VIR_DOMAIN_CRASHED_UNKNOWN = 0, /* crashed for unknown reason */ +VIR_DOMAIN_CRASHED_PANICKED = 1, /* domain panicked */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_CRASHED_LAST @@ -3319,6 +3323,7 @@ typedef enum { VIR_DOMAIN_EVENT_STOPPED = 5, VIR_DOMAIN_EVENT_SHUTDOWN = 6, VIR_DOMAIN_EVENT_PMSUSPENDED = 7, +VIR_DOMAIN_EVENT_CRASHED = 8, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_LAST @@ -3450,6 +3455,17 @@ typedef enum { #endif } virDomainEventPMSuspendedDetailType; +/* + * Details about the 'crashed' lifecycle event + */ +typedef enum { +VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0, /* Guest was panicked */ + +#ifdef VIR_ENUM_SENTINELS +VIR_DOMAIN_EVENT_CRASHED_LAST +#endif +} virDomainEventCrashedDetailType; + /** * virConnectDomainEventCallback: * @conn: virConnect connection diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..f577fd4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -642,7 +642,8 @@ VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST, unpaused, migration canceled, save canceled, - wakeup) + wakeup, + from crashed) VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST, unknown) @@ -657,11 +658,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, watchdog, from snapshot, shutdown, - snapshot) + snapshot, + guest panicked) VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST, unknown, - user) + user, + crashed) VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, unknown, @@ -674,7 +677,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, from snapshot) VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, - unknown) + unknown, + panicked) VIR_ENUM_IMPL(virDomainPMSuspendedReason, VIR_DOMAIN_PMSUSPENDED_LAST, unknown) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.
This bug will cause the dest get a unexpected value. Example: [root@yds-pc libvirt]# virsh Welcome to virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # connect error: Failed to connect to the hypervisor error: internal error Unable to parse URI �N�*C Please someone give me a comment. Thanx~ On 05/28/2013 03:13 PM, yangdongsheng wrote: When src is NULL, vir_strdup will return 0 directly. This patch will set dest to NULL before vir_strdup return. Signed-off-by: yangdongshengyangds.f...@cn.fujitsu.com --- src/util/virstring.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index b244e6c..25b5d81 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -540,8 +540,10 @@ virStrdup(char **dest, const char *funcname, size_t linenr) { -if (!src) +if (!src) { +*dest = NULL; return 0; +} if (!(*dest = strdup(src))) { if (report) virReportOOMErrorFull(domcode, filename, funcname, linenr); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.
For qemu, if the -smp N or the value of maxcpus is given, it define the number of vcpu of guest whenever the vcpu topology is defined or not. But if the -smp N and maxcpus are missing, the topology can compute and define vcpus for guest automatically by math: vcpu number = sockets*cores*threads For libvirt, as vcpu is always mandatory, so we can ask topology to match maximum vcpu numbers. --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..ffdc6da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (def-cpu-sockets -def-maxvcpus +def-maxvcpus != def-cpu-sockets * def-cpu-cores * def-cpu-threads) { virReportError(VIR_ERR_XML_DETAIL, %s, - _(Maximum CPUs greater than topology limit)); + _(Topology limit does not match maximum CPUs)); goto error; } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface
Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 59 +++- tools/virsh.pod | 34 +- 2 files changed, 60 insertions(+), 33 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4de5dd5..96f6765 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -724,14 +724,6 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_DATA, .help = N_(model type) }, -{.name = persistent, - .type = VSH_OT_ALIAS, - .help = config -}, -{.name = config, - .type = VSH_OT_BOOL, - .help = N_(affect next boot) -}, {.name = inbound, .type = VSH_OT_DATA, .help = N_(control domain's incoming traffics) @@ -740,6 +732,22 @@ static const vshCmdOptDef opts_attach_interface[] = { .type = VSH_OT_DATA, .help = N_(control domain's outgoing traffics) }, +{.name = persistent, + .type = VSH_OT_BOOL, + .help = N_(make live change persistent) +}, +{.name = config, + .type = VSH_OT_BOOL, + .help = N_(affect next boot) +}, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, {.name = NULL} }; @@ -789,12 +797,30 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) int typ; int ret; bool functionReturn = false; -unsigned int flags; virBuffer buf = VIR_BUFFER_INITIALIZER; char *xml; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; +bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool persistent = vshCommandOptBool(cmd, persistent); + +VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + +VSH_EXCLUSIVE_OPTIONS_VAR(current, live); +VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + +if (config || persistent) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) -goto cleanup; +return false; + +if (persistent +virDomainIsActive(dom) == 1) +flags |= VIR_DOMAIN_AFFECT_LIVE; if (vshCommandOptStringReq(ctl, cmd, type, type) 0 || vshCommandOptStringReq(ctl, cmd, source, source) 0 || @@ -887,14 +913,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) xml = virBufferContentAndReset(buf); -if (vshCommandOptBool(cmd, config)) { -flags = VIR_DOMAIN_AFFECT_CONFIG; -if (virDomainIsActive(dom) == 1) -flags |= VIR_DOMAIN_AFFECT_LIVE; +if (flags) ret = virDomainAttachDeviceFlags(dom, xml, flags); -} else { +else ret = virDomainAttachDevice(dom, xml); -} VIR_FREE(xml); @@ -905,9 +927,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) functionReturn = true; } - cleanup: -if (dom) -virDomainFree(dom); +cleanup: +virDomainFree(dom); virBufferFreeAndReset(buf); return functionReturn; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 9ae96d1..0fd546d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1885,25 +1885,31 @@ For compatibility purposes, I--persistent behaves like I--config for an offline domain, and like I--live I--config for a running domain. =item Battach-interface Idomain Itype Isource +[[[I--live] [I--config] | [I--current]] | [I--persistent]] [I--target target] [I--mac mac] [I--script script] [I--model model] [I--config] [I--inbound average,peak,burst] [I--outbound average,peak,burst] -Attach a new network interface to the domain. -Itype can be either Inetwork to indicate a physical network device or -Ibridge to indicate a bridge to a device. -Isource indicates the source device. -Itarget allows to indicate the target device in the guest. Names starting -with 'vnet' are considered as auto-generated an hence blanked out. -Imac allows to specify the MAC address of the network interface. -Iscript allows to specify a path to a script handling a bridge instead of -the default one. -Imodel allows to specify the model type. -I--config indicates the changes will affect the next boot of the domain, -for compatibility purposes, I--persistent is alias of I--config. -Iinbound and Ioutbound control the bandwidth of the interface. Ipeak -and Iburst are optional, so average,peak, average,,burst and +Attach a new network interface to the domain. Itype can be either Inetwork +to indicate a physical network device or Ibridge to indicate a bridge to a +device. Isource indicates the source device. Itarget allows to indicate +the target device in the guest. Names starting with 'vnet' are considered as +auto-generated an hence blanked out. Imac
[libvirt] [PATCH 0/3] virsh: Add --live, --config, --current to attach-* commands too
In commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d a pattern was established to upgrade hotplug commands in virsh. The series containing the patch forgot to upgrade the attach-* commands too. Peter Krempa (3): virsh-domain: Add --live, --config, --current logic to cmdAttachDevice virsh-domain: Add --live, --config, --current logic to cmdAttachDisk virsh-domain: Add --live, --config, --current logic to cmdAttachInterface tools/virsh-domain.c | 153 --- tools/virsh.pod | 63 ++--- 2 files changed, 153 insertions(+), 63 deletions(-) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDisk
Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 53 tools/virsh.pod | 13 +++-- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b965c11..4de5dd5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -299,14 +299,6 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_STRING, .help = N_(mode of device reading and writing) }, -{.name = persistent, - .type = VSH_OT_ALIAS, - .help = config -}, -{.name = config, - .type = VSH_OT_BOOL, - .help = N_(affect next boot) -}, {.name = sourcetype, .type = VSH_OT_STRING, .help = N_(type of source (block|file)) @@ -335,7 +327,22 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_BOOL, .help = N_(print XML document rather than attach the disk) }, - +{.name = persistent, + .type = VSH_OT_BOOL, + .help = N_(make live change persistent) +}, +{.name = config, + .type = VSH_OT_BOOL, + .help = N_(affect next boot) +}, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, {.name = NULL} }; @@ -496,15 +503,33 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; -unsigned int flags; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *xml = NULL; struct stat st; +bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool persistent = vshCommandOptBool(cmd, persistent); + +VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + +VSH_EXCLUSIVE_OPTIONS_VAR(current, live); +VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + +if (config || persistent) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; +if (persistent +virDomainIsActive(dom) == 1) +flags |= VIR_DOMAIN_AFFECT_LIVE; + if (vshCommandOptStringReq(ctl, cmd, source, source) 0 || vshCommandOptStringReq(ctl, cmd, target, target) 0 || vshCommandOptStringReq(ctl, cmd, driver, driver) 0 || @@ -635,14 +660,10 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) goto cleanup; } -if (vshCommandOptBool(cmd, config)) { -flags = VIR_DOMAIN_AFFECT_CONFIG; -if (virDomainIsActive(dom) == 1) -flags |= VIR_DOMAIN_AFFECT_LIVE; +if (flags) ret = virDomainAttachDeviceFlags(dom, xml, flags); -} else { +else ret = virDomainAttachDevice(dom, xml); -} if (ret != 0) { vshError(ctl, %s, _(Failed to attach disk)); diff --git a/tools/virsh.pod b/tools/virsh.pod index a002623..9ae96d1 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1839,6 +1839,7 @@ results as some fields may be autogenerated and thus match devices other than expected. =item Battach-disk Idomain Isource Itarget +[[[I--live] [I--config] | [I--current]] | [I--persistent]] [I--driver driver] [I--subdriver subdriver] [I--cache cache] [I--type type] [I--mode mode] [I--config] [I--sourcetype soucetype] [I--serial serial] [I--shareable] [I--rawio] [I--address address] @@ -1859,8 +1860,6 @@ alternative to the disk default, although this use only replaces the media within the existing virtual cdrom or floppy device; consider using Bupdate-device for this usage instead. Imode can specify the two specific mode Ireadonly or Ishareable. -I--config indicates the changes will affect the next boot of the domain, -for compatibility purposes, I--persistent is alias of I--config. Isourcetype can indicate the type of source (block|file) Icache can be one of default, none, writethrough, writeback, directsync or unsafe. @@ -1875,6 +1874,16 @@ address. If I--print-xml is specified, then the XML of the disk that would be attached is printed instead. +If I--live is specified, affect a running domain. +If I--config is specified, affect the next startup of a persistent domain. +If I--current is specified, affect the current domain state. +Both I--live and I--config flags may be given, but I--current is +exclusive. When no flag is specified legacy API is used whose behavior depends +on the hypervisor driver. + +For compatibility purposes, I--persistent behaves like I--config for +an offline domain, and like I--live I--config for a running domain. + =item Battach-interface Idomain Itype Isource [I--target target]
[libvirt] [PATCH 1/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDevice
Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 41 - tools/virsh.pod | 16 +--- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0402aef..b965c11 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -173,13 +173,21 @@ static const vshCmdOptDef opts_attach_device[] = { .help = N_(XML file) }, {.name = persistent, - .type = VSH_OT_ALIAS, - .help = config + .type = VSH_OT_BOOL, + .help = N_(make live change persistent) }, {.name = config, .type = VSH_OT_BOOL, .help = N_(affect next boot) }, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, {.name = NULL} }; @@ -191,7 +199,21 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) char *buffer; int rv; bool ret = false; -unsigned int flags; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; +bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool persistent = vshCommandOptBool(cmd, persistent); + +VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + +VSH_EXCLUSIVE_OPTIONS_VAR(current, live); +VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + +if (config || persistent) +flags |= VIR_DOMAIN_AFFECT_CONFIG; +if (live) +flags |= VIR_DOMAIN_AFFECT_LIVE; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -199,19 +221,20 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, file, from) 0) goto cleanup; +if (persistent +virDomainIsActive(dom) == 1) +flags |= VIR_DOMAIN_AFFECT_LIVE; + if (virFileReadAll(from, VSH_MAX_XML_FILE, buffer) 0) { vshReportError(ctl); goto cleanup; } -if (vshCommandOptBool(cmd, config)) { -flags = VIR_DOMAIN_AFFECT_CONFIG; -if (virDomainIsActive(dom) == 1) - flags |= VIR_DOMAIN_AFFECT_LIVE; +if (flags) rv = virDomainAttachDeviceFlags(dom, buffer, flags); -} else { +else rv = virDomainAttachDevice(dom, buffer); -} + VIR_FREE(buffer); if (rv 0) { diff --git a/tools/virsh.pod b/tools/virsh.pod index 11984bc..a002623 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1809,7 +1809,8 @@ format of the device sections to get the most accurate set of accepted values. =over 4 -=item Battach-device Idomain IFILE [I--config] +=item Battach-device Idomain IFILE +[[[I--live] [I--config] | [I--current]] | [I--persistent]] Attach a device to the domain, using a device definition in an XML file using a device definition element such as disk or interface @@ -1817,13 +1818,22 @@ as the top-level element. See the documentation at Lhttp://libvirt.org/formatdomain.html#elementsDevices to learn about libvirt XML format for a device. If I--config is specified the command alters the persistent domain configuration with the device -attach taking effect the next time libvirt starts the domain. For -compatibility purposes, I--persistent is an alias of I--config. +attach taking effect the next time libvirt starts the domain. For cdrom and floppy devices, this command only replaces the media within an existing device; consider using Bupdate-device for this usage. For passthrough host devices, see also Bnodedev-detach, needed if the device does not use managed mode. +If I--live is specified, affect a running domain. +If I--config is specified, affect the next startup of a persistent domain. +If I--current is specified, affect the current domain state. +Both I--live and I--config flags may be given, but I--current is +exclusive. When no flag is specified legacy API is used whose behavior depends +on the hypervisor driver. + +For compatibility purposes, I--persistent behaves like I--config for +an offline domain, and like I--live I--config for a running domain. + BNote: using of partial device definition XML files may lead to unexpected results as some fields may be autogenerated and thus match devices other than expected. -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/3] libvirt supports Guest panicked
On 05/28/2013 11:00 AM, Chen Fan wrote: This patchs implement the 'on_crash' behavior in the XML example XML: on_crashdestroy/on_crash Changes: v2-v3: 1. split into 3 patches v1-v2: 1. fix the incorrect domain state: paused - crashed, when crash the guest while libvirt isn't running, then restart libvirtd. Chen Fan (3): libvirt: Define domain crash event types qemu: Supports guest panicked virsh: supports guest panicked examples/domain-events/events-c/event-test.c | 10 +++ include/libvirt/libvirt.h.in | 16 + src/conf/domain_conf.c | 12 ++-- src/qemu/qemu_monitor.c | 14 +++- src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 7 ++ src/qemu/qemu_process.c | 103 ++- tools/virsh-domain-monitor.c | 8 +++ 8 files changed, 169 insertions(+), 6 deletions(-) Applied and tested on s390. Works fine and as expected (by me). Of course Daniel or Eric should have a final look at it, as this series extends the domain state model. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] storage_conf: Improve the memory deallocation of virStorageVolDefParseXML
On 05/22/2013 02:05 PM, Osier Yang wrote: Changes: * Add a new goto label error * Free the strings at cleanup * Remove the unnecessary frees --- src/conf/storage_conf.c | 29 ++--- 1 file changed, 14 insertions(+), 15 deletions(-) ACK. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] storage_conf: Improve error messages
On 05/22/2013 02:05 PM, Osier Yang wrote: virStoragePoolDefParseSource: * Better error message virStoragePoolObjLoad: * Break the line line --- src/conf/storage_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 44ecb2a..a62629e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -531,7 +531,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, source-name = virXPathString(string(./name), ctxt); if (pool_type == VIR_STORAGE_POOL_RBD source-name == NULL) { virReportError(VIR_ERR_XML_ERROR, %s, - _(missing mandatory 'name' field for RBD pool name)); + _(element 'name' is mandatory for RBD pool name)); s/pool name/pools/ goto cleanup; } @@ -1695,7 +1695,8 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, if (!virFileMatchesNameSuffix(file, def-name, .xml)) { virReportError(VIR_ERR_XML_ERROR, - _(Storage pool config filename '%s' does not match pool name '%s'), + _(Storage pool config filename '%s' does + not match pool name '%s'), path, def-name); virStoragePoolDefFree(def); return NULL; ACK Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] storage_conf: Improve the memory deallocation of pool def parsing
On 05/22/2013 02:05 PM, Osier Yang wrote: Changes: * Free all the strings at cleanup, instead of freeing them in the middle * Remove xmlFree * s/tmppath/target_path/, to make it more sensible * Add new goto label error --- src/conf/storage_conf.c | 54 - 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index efe02e8..6f0ed74 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -914,7 +910,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) virReportError(VIR_ERR_XML_ERROR, %s, _('wwnn' and 'wwpn' must be specified for adapter type 'fchost')); -goto cleanup; +goto error; } if (!virValidateWWN(ret-source.adapter.data.fchost.wwnn) || If the WWN validation fails here, you still jump to cleanup instead of error. ACK if you fix that. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] bridge driver: don't masquerade local subnet broadcast/multicast packets
I'm not sure if everyone that needs to see this discussion are subscribed to RH bugzilla bug #709418 so I will repeat my comments from comment #12 on that bug here: On 13-05-27 10:34 PM, Laszlo Ersek wrote: Packets sent by guests on virbrN, *or* by dnsmasq on the same, to - 255.255.255.255/32 (netmask-independent local network broadcast address), or to - 224.0.0.0/24 (local subnetwork multicast range) Multicast is much wider than just 224.0.0.0/24. Per my patch in attachment 526549 [details], it's in fact 224.0.0.0/4. All multicast needs to be exempt from NAT. Quoting from Wikipedia article http://en.wikipedia.org/wiki/Multicast_address: IPv4 multicast addresses are defined by the leading address bits of 1110, originating from the classful network design of the early Internet when this group of addresses was designated as Class D. The Classless Inter-Domain Routing (CIDR) prefix of this group is 224.0.0.0/4. Indeed, Wikipedia is not authoritative, but it does provide a good starting point to finding authoritative information if you want. I suppose the authoritative doc is at http://www.iana.org/assignments/multicast-addresses/multicast-addresses.xml or somesuch. It does not explicitly say 224.0.0.0/4 but it certainly implies it in discussing the entire space from 224.0.0.0 through 239.255.255.255. diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d5886fe..2b3b250 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1542,6 +1542,9 @@ networkRefreshDaemons(struct network_driver *driver) } } +static const char networkLocalMulticast[] = 224.0.0.0/24; This should be /8, not /24. There are many multicast protocols that operate outside of that 224.0.0.0/24 network. In fact they all must. 224.0.0.0/24 is reserved and assignment must be made from IANA. Let me give you a scenario where your 224.0.0.0/24 fails to correct the problem: Imagine you have a cluster of VMs all with a network address of 192.168.122.[2-10]/24 and 192.168.122.1 is your VM server. Now imagine those VMs are using a multicast address of 226.1.2.3 (just for argument's sake). Each node will be listening on 226.1.2.3 for other members of the group to build a list of members. But once the packet from (say) 192.168.122.2-226.1.2.3 goes through the VM host's NAT it will look to the other group members to be from host 192.168.122.1, so they all add 192.168.122.1 to their member list. Then 192.168.122.3 sends it's packet to 226.1.2.3, it goes through NAT also and all of the group members notice they already have 192.168.122.1 in their group and just ignore it. That goes on for all of the [2-10] nodes and none of them get added to the group list. That's because NAT broke the protocol. Further, to your arguments in comment #13 in BZ #709418 about needing to map ports 1023 for security: The source port mapping goal has some specific parameters to it. It's trying to avoid a VM from masquerading as it's host and leveraging it's host's permission in the network. But that's only a problem because the VM is masquerading as the host. When you exempt multicast from masquerading you no longer have the problem of the VM being mistaken for the host and therefore you don't need the quasi-security of not allowing the VM to use ports 1024. Put another way, mapping the source port numbers is only a requirement because the VM is borrowing the identity of the host. Once you eliminate the identity borrowing you can eliminate the need for the safety guards that that lending is putting in place. Cheers, b. signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDevice
On 05/28/2013 06:52 PM, Peter Krempa wrote: Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 41 - tools/virsh.pod | 16 +--- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0402aef..b965c11 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -173,13 +173,21 @@ static const vshCmdOptDef opts_attach_device[] = { .help = N_(XML file) }, {.name = persistent, - .type = VSH_OT_ALIAS, - .help = config + .type = VSH_OT_BOOL, + .help = N_(make live change persistent) }, {.name = config, .type = VSH_OT_BOOL, .help = N_(affect next boot) }, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, {.name = NULL} }; @@ -191,7 +199,21 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) char *buffer; int rv; bool ret = false; -unsigned int flags; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; +bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool persistent = vshCommandOptBool(cmd, persistent); + +VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); VSH_EXCLUSIVE_OPTIONS_VAR(current, persistent) could be a little more consistent with the following two lines. + +VSH_EXCLUSIVE_OPTIONS_VAR(current, live); +VSH_EXCLUSIVE_OPTIONS_VAR(current, config); ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] virsh-domain: Add --live, --config, --current logic to cmdAttachDisk
On 05/28/2013 06:52 PM, Peter Krempa wrote: Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 53 tools/virsh.pod | 13 +++-- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b965c11..4de5dd5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -299,14 +299,6 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_STRING, .help = N_(mode of device reading and writing) }, -{.name = persistent, - .type = VSH_OT_ALIAS, - .help = config -}, -{.name = config, - .type = VSH_OT_BOOL, - .help = N_(affect next boot) -}, {.name = sourcetype, .type = VSH_OT_STRING, .help = N_(type of source (block|file)) @@ -335,7 +327,22 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_BOOL, .help = N_(print XML document rather than attach the disk) }, - +{.name = persistent, + .type = VSH_OT_BOOL, + .help = N_(make live change persistent) +}, +{.name = config, + .type = VSH_OT_BOOL, + .help = N_(affect next boot) +}, +{.name = live, + .type = VSH_OT_BOOL, + .help = N_(affect running domain) +}, +{.name = current, + .type = VSH_OT_BOOL, + .help = N_(affect current domain) +}, {.name = NULL} }; @@ -496,15 +503,33 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) struct DiskAddress diskAddr; bool isFile = false, functionReturn = false; int ret; -unsigned int flags; +unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *xml = NULL; struct stat st; +bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(cmd, config); +bool live = vshCommandOptBool(cmd, live); +bool persistent = vshCommandOptBool(cmd, persistent); + +VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current); + ACK either change or not^. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util/vshstring: fix the vir_strdup when src is NULL.
On 05/28/2013 01:13 AM, yangdongsheng wrote: When src is NULL, vir_strdup will return 0 directly. This patch will set dest to NULL before vir_strdup return. Signed-off-by: yangdongsheng yangds.f...@cn.fujitsu.com --- src/util/virstring.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ACK, but VIR_STRNDUP has the same problem. I'm pushing this with an updated commit message (your followup mail that shows where it goes wrong if not set to NULL) and with both problems fixed. diff --git a/src/util/virstring.c b/src/util/virstring.c index b244e6c..25b5d81 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -540,8 +540,10 @@ virStrdup(char **dest, const char *funcname, size_t linenr) { -if (!src) +if (!src) { +*dest = NULL; return 0; +} if (!(*dest = strdup(src))) { if (report) virReportOOMErrorFull(domcode, filename, funcname, linenr); -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface
On 05/28/2013 06:52 PM, Peter Krempa wrote: Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 59 +++- tools/virsh.pod | 34 +- 2 files changed, 60 insertions(+), 33 deletions(-) -Attach a new network interface to the domain. -Itype can be either Inetwork to indicate a physical network device or -Ibridge to indicate a bridge to a device. -Isource indicates the source device. -Itarget allows to indicate the target device in the guest. Names starting -with 'vnet' are considered as auto-generated an hence blanked out. -Imac allows to specify the MAC address of the network interface. -Iscript allows to specify a path to a script handling a bridge instead of -the default one. -Imodel allows to specify the model type. -I--config indicates the changes will affect the next boot of the domain, -for compatibility purposes, I--persistent is alias of I--config. -Iinbound and Ioutbound control the bandwidth of the interface. Ipeak -and Iburst are optional, so average,peak, average,,burst and +Attach a new network interface to the domain. Itype can be either Inetwork +to indicate a physical network device or Ibridge to indicate a bridge to a +device. Isource indicates the source device. Itarget allows to indicate +the target device in the guest. Names starting with 'vnet' are considered as +auto-generated an hence blanked out. Sorry the last sentence I can't quite follow it. ACK with the rest. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest
The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 4096 cpus on the host with the built-in maximum of 256 cpus per guest. The full cpu map of such a system takes 128 kilobytes and the map for vcpu pinning is 512 bytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 1ebbce7..a94e0db 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -82,13 +82,13 @@ const REMOTE_DOMAIN_ID_LIST_MAX = 16384; const REMOTE_DOMAIN_NAME_LIST_MAX = 16384; /* Upper limit on cpumap (bytes) passed to virDomainPinVcpu. */ -const REMOTE_CPUMAP_MAX = 256; +const REMOTE_CPUMAP_MAX = 512; /* Upper limit on number of info fields returned by virDomainGetVcpus. */ -const REMOTE_VCPUINFO_MAX = 2048; +const REMOTE_VCPUINFO_MAX = 4096; /* Upper limit on cpumaps (bytes) passed to virDomainGetVcpus. */ -const REMOTE_CPUMAPS_MAX = 16384; +const REMOTE_CPUMAPS_MAX = 131072; /* Upper limit on migrate cookie. */ const REMOTE_MIGRATE_COOKIE_MAX = 16384; -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest
On 05/28/2013 06:35 AM, Peter Krempa wrote: The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 4096 cpus on the host with the built-in maximum of 256 cpus per guest. The full cpu map of such a system takes 128 kilobytes and the map for vcpu pinning is 512 bytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) We missed rc1 - is this still worth getting into 1.0.6? I'm not opposed to the change, just questioning the timing of it. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: allow for greater capacity/allocation
On 05/27/2013 01:35 PM, Guido Günther wrote: Don't be too picky about the actual size and allocation. The size needs to be correct but it's o.k. for allocation and size to be bigger. Debian Wheezy's qemu adds 4096 bytes. Yeah, different versions of qemu round to different granularity; rather annoying but that's life. --- scripts/domain/121-block-info.t |8 1 file changed, 4 insertions(+), 4 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: omit network
On 05/27/2013 01:17 PM, Guido Günther wrote: qemu:///session doesn't have a default network so we fail with: ./scripts/domain/121-block-info.t .. 1/29 # Defining transient storage pool # Generic guest with pervious created vol Is this typo (s/pervious/previous/) something that also needs fixing in source? ./scripts/domain/121-block-info.t .. 12/29 # Failed test 'Create domain' # at /var/lib/jenkins/jobs/libvirt-tck-build/workspace/lib//Sys/Virt/TCK.pm line 803. # expected Sys::Virt::Domain object # found 'libvirt error code: 43, message: Network not found: no network with matching name 'default' # ' The network isn't needed so simply leave it out. --- scripts/domain/121-block-info.t |1 - 1 file changed, 1 deletion(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] snapshot-create-as Permission denied
On 05/26/2013 08:56 PM, yue wrote: [please don't top-post on technical lists] hi. my environment: centos 6.3, qemu 1.5(source code build), libvirt libvirt-0.10.2-18.el6_4.2.x86_64.selinux enforce . It's best to use the entire stack from your distro, or to self-build the entire stack. Mixing newer qemu with older libvirt might have unexpected consequences, and since you are using CentOS, you have no one to blame but yourself. We are unable to help you here unless you can reproduce the problem with the latest libvirt. i have 2 questions 1.snapshot. permisson deny. dumpxml: seclabel type='dynamic' model='selinux' relabel='yes' labelsystem_u:system_r:svirt_t:s0:c33,c172/label imagelabelsystem_u:object_r:svirt_image_t:s0:c33,c172/imagelabel /seclabel command line: [root@ovirtdev images]# ls -lZ -rw-r--r--. qemu qemu system_u:object_r:virt_image_t:s0 test.qcow2 image does not have the same MLS? it does not seem like a selinux problem, because selinix does not record this deny. There have been bug fixes in upstream libvirt related to permissions on snapshot creation, although it's hard to say whether all of those have been backported into the downstream version of libvirt that you are using. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RPC: Support up to 4096 cpus on the host and 256 in the guest
On 05/28/13 14:50, Eric Blake wrote: On 05/28/2013 06:35 AM, Peter Krempa wrote: The RPC limits for cpu maps didn't allow to use libvirt on ultra big boxes. This patch increases size of the limits to support a maximum of 4096 cpus on the host with the built-in maximum of 256 cpus per guest. The full cpu map of such a system takes 128 kilobytes and the map for vcpu pinning is 512 bytes long. --- src/remote/remote_protocol.x | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) We missed rc1 - is this still worth getting into 1.0.6? I'm not opposed to the change, just questioning the timing of it. I don't mind waiting until after the release to push this. Anyways this should be safe as we are just raising the limits and with some trickery I was able to verify that RPC is okay with transporting this amount of information this by reporting fake counts of cpus. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't report error on successful media eject
If we are just ejecting media, ret == -1 even after the retry loop determines that the tray is open, as requested. This means media disconnect always report's error. Fix it, and fix some other mini issues: - Don't overwrite the 'eject' error message if the retry loop fails - Move the retries decrement inside the loop, otherwise the final loop might succeed, yet retries == 0 and we will raise error - Setting ret = -1 in the disk-src check is unneeded - Fix comment typos cc: mpriv...@redhat.com --- src/qemu/qemu_hotplug.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1ed61db..3b277ef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -98,10 +98,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ -while (retries--) { +while (retries) { if (origdisk-tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) break; +retries--; virObjectUnlock(vm); VIR_DEBUG(Waiting 500ms for tray to open. Retries left %d, retries); usleep(500 * 1000); /* sleep 500ms */ @@ -109,19 +110,20 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } virObjectUnref(vm); -if (disk-src) { -/* deliberately don't depend on 'ret' as 'eject' may have failed for the - * fist time and we are gonna check the drive state anyway */ -const char *format = NULL; - -/* We haven't succeeded yet */ -ret = -1; - -if (retries = 0) { +if (retries = 0) { +if (ret == 0) { +/* If ret == -1, EjectMedia already set an error message */ virReportError(VIR_ERR_OPERATION_FAILED, %s, - _(Unable to eject media before changing it)); -goto audit; + _(Unable to eject media)); } +goto audit; +} +ret = 0; + +if (disk-src) { +/* deliberately don't depend on 'ret' as 'eject' may have failed the + * first time and we are gonna check the drive state anyway */ +const char *format = NULL; if (disk-type != VIR_DOMAIN_DISK_TYPE_DIR) { if (disk-format 0) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon
On 05/25/2013 03:13 PM, Martin Kletzander wrote: @@ -31,6 +32,9 @@ foreach my $file (@ARGV) { while (defined (my $line = FILE)) { my $data = $line; +# Kill any quoted ; or +$data =~ s,'[;]','X',g; + Good catch, I didn't know this was missing. Yep, I hit some false positives where we parse for ';' if I didn't add this. [...] ACK, Thanks; pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] conf: Generate address for scsi host device automatically
On 05/23/2013 12:48 PM, Osier Yang wrote: With unknown good reasons, the attribute bus of scsi device address is always set to 0, same for attribute target. (See virDomainDiskDefAssignAddress). Though we might need to change the algorithm to honor bus and target too, that's a different issue. The address generator for scsi host device in this patch just follows the unknown good reasons, only considering the controller and unit. It walks through all scsi controllers and their units, to see if the address $controller:0:0:$unit can be used, if found one, it sits on it, otherwise, it creates a new controller (actually the controller is implicitly created by someone else), and sits on $new_controller:0:0:0 instead. --- v1 - v2: * A new helper virDomainSCSIDriveAddressIsUsed * Move virDomainDefMaybeAddHostdevSCSIcontroller * More comments * Add testing. * problems fixed with the testing: 1) s/nscsi_controllers + 1/nscsi_controllers/, 2) Add the controller implicitly after a scsi hostdev def parsing, as it can use a new scsi controller index (nscsi_controllers), which should be added implicitly. First pass of code is found at: https://www.redhat.com/archives/libvir-list/2013-May/msg00340.html The previous/initial review and reply is found at: https://www.redhat.com/archives/libvir-list/2013-May/msg00506.html https://www.redhat.com/archives/libvir-list/2013-May/msg00526.html --- src/conf/domain_conf.c | 209 ++--- .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 95 ++ ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++ tests/qemuxml2xmltest.c| 2 + 4 files changed, 382 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ad5550c..86d743b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3782,6 +3782,148 @@ cleanup: return ret; } +/* Check if a drive type address $controller:0:0:$unit is already + * taken by a disk or not. + */ +static bool +virDomainDriveAddressIsUsedByDisk(virDomainDefPtr def, + enum virDomainDiskBus type, + unsigned int controller, + unsigned int unit) +{ +virDomainDiskDefPtr disk; +int i; + +for (i = 0; i def-ndisks; i++) { +disk = def-disks[i]; + +if (disk-bus != type || +disk-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) +continue; + +if (disk-info.addr.drive.controller == controller +disk-info.addr.drive.unit == unit +disk-info.addr.drive.bus == 0 +disk-info.addr.drive.target == 0) +return true; +} + +return false; +} + +/* Check if a drive type address $controller:0:0:$unit is already + * taken by a host device or not. + */ +static bool +virDomainDriveAddressIsUsedByHostdev(virDomainDefPtr def, + enum virDomainHostdevSubsysType type, + unsigned int controller, + unsigned int unit) +{ +virDomainHostdevDefPtr hostdev; +int i; + +for (i = 0; i def-nhostdevs; i++) { +hostdev = def-hostdevs[i]; + +if (hostdev-source.subsys.type != type) +continue; + +if (hostdev-info-addr.drive.controller == controller +hostdev-info-addr.drive.unit == unit +hostdev-info-addr.drive.bus == 0 +hostdev-info-addr.drive.target == 0) +return true; +} + +return false; +} + +static bool +virDomainSCSIDriveAddressIsUsed(virDomainDefPtr def, +unsigned int controller, +unsigned int unit) +{ +/* In current implementation, the maximum unit number of a controller + * is either 16 or 7 (narrow SCSI bus), and if the maximum unit number + * is 16, the controller itself is on unit 7 */ +if (unit == 7) FYI: The previous code cared that a wide address was being used when returning true - this code doesn't. Just making sure that change is intentional. Essentially, 7 cannot be used. The comments above the code talk about wide/narrow which it seems becomes irrelevant. The reality seems to be that you are avoiding using 7 altogether because it's reserved for the controller itself, correct? Regardless of whether a wide or narrow. The controller itself uses the unit number of the maximum value of the narrow band. At least that's how I'm reading it. +return true; + +if
Re: [libvirt] [libvirt-tck PATCH] 121-block-info.t: omit network
On Tue, May 28, 2013 at 06:56:21AM -0600, Eric Blake wrote: On 05/27/2013 01:17 PM, Guido Günther wrote: qemu:///session doesn't have a default network so we fail with: ./scripts/domain/121-block-info.t .. 1/29 # Defining transient storage pool # Generic guest with pervious created vol Is this typo (s/pervious/previous/) something that also needs fixing in source? ./scripts/domain/121-block-info.t .. 12/29 # Failed test 'Create domain' # at /var/lib/jenkins/jobs/libvirt-tck-build/workspace/lib//Sys/Virt/TCK.pm line 803. # expected Sys::Virt::Domain object # found 'libvirt error code: 43, message: Network not found: no network with matching name 'default' # ' The network isn't needed so simply leave it out. --- scripts/domain/121-block-info.t |1 - 1 file changed, 1 deletion(-) ACK. Both pushed including the typo fix. Thanks! -- Guido -- Eric Blake eblake redhat com+1-919-301-3266 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 3/3] virsh-domain: Add --live, --config, --current logic to cmdAttachInterface
On 05/28/13 14:01, Guannan Ren wrote: On 05/28/2013 06:52 PM, Peter Krempa wrote: Use the approach established in commit 69ce3ffa8d431e9810607c6e00b7cfcc481b491d to improve this function too. --- tools/virsh-domain.c | 59 +++- tools/virsh.pod | 34 +- 2 files changed, 60 insertions(+), 33 deletions(-) -Attach a new network interface to the domain. -Itype can be either Inetwork to indicate a physical network device or -Ibridge to indicate a bridge to a device. -Isource indicates the source device. -Itarget allows to indicate the target device in the guest. Names starting -with 'vnet' are considered as auto-generated an hence blanked out. -Imac allows to specify the MAC address of the network interface. -Iscript allows to specify a path to a script handling a bridge instead of -the default one. -Imodel allows to specify the model type. -I--config indicates the changes will affect the next boot of the domain, -for compatibility purposes, I--persistent is alias of I--config. -Iinbound and Ioutbound control the bandwidth of the interface. Ipeak -and Iburst are optional, so average,peak, average,,burst and +Attach a new network interface to the domain. Itype can be either Inetwork +to indicate a physical network device or Ibridge to indicate a bridge to a +device. Isource indicates the source device. Itarget allows to indicate +the target device in the guest. Names starting with 'vnet' are considered as +auto-generated an hence blanked out. Sorry the last sentence I can't quite follow it. Hm. I actually didn't change the last sentence, I just reflowed the paragraph and deleted the sentence about --config and --persistent. The last sentence means that if you use a interface name starting with 'vnet' it will vanish as it's considered internal and autogenerated. ACK with the rest. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't report error on successful media eject
On 05/28/2013 08:37 AM, Cole Robinson wrote: If we are just ejecting media, ret == -1 even after the retry loop determines that the tray is open, as requested. This means media disconnect always report's error. Fix it, and fix some other mini issues: - Don't overwrite the 'eject' error message if the retry loop fails - Move the retries decrement inside the loop, otherwise the final loop might succeed, yet retries == 0 and we will raise error - Setting ret = -1 in the disk-src check is unneeded - Fix comment typos cc: mpriv...@redhat.com --- src/qemu/qemu_hotplug.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) ACK. + +if (disk-src) { +/* deliberately don't depend on 'ret' as 'eject' may have failed the + * first time and we are gonna check the drive state anyway */ Comment motion, but might be worth s/gonna/going to/ while touching it (I'm actually surprised that thunderbird didn't flag the slang 'gonna' as a misspelled word). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon
On 25/05/13 01:19, Eric Blake wrote: Enforce the style cleanup in the previous patch. * build-aux/bracket-spacing.pl: Enforce trailing spacing. * cfg.mk (bracket-spacing-check): Tweak error wording. * docs/hacking.html.in: Document the rule. * HACKING: Regenerate. Signed-off-by: Eric Blake ebl...@redhat.com --- HACKING | 23 +++ build-aux/bracket-spacing.pl | 12 cfg.mk | 2 +- docs/hacking.html.in | 29 + 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/HACKING b/HACKING index 2bd6d69..42f76b6 100644 --- a/HACKING +++ b/HACKING @@ -318,6 +318,29 @@ immediately prior to any closing bracket. E.g. int foo(int wizz);// Good +Semicolons +== +Semicolons should never have a space beforehand. Inside the condition of a +for loop, there should always be a space or line break after each semicolon, +except for the special case of an infinite loop (although more infinite loops +use while). While not enforced, loop counters generally use post-increment. + + for (i = 0 ;i limit ; ++i) { // Bad + for (i = 0; i limit; i++) { // Good + for (;;) { // ok + while (1) { // Better + +Empty loop bodies are better represented with curly braces and a comment, +although use of a semicolon is not currently rejected. + + while ((rc = waitpid(pid, st, 0) == -1) // ok + errno == EINTR); + while ((rc = waitpid(pid, st, 0) == -1) // Better + errno == EINTR) { + /* nothing */ + } + + Curly braces Omit the curly braces around an if, while, for etc. body only when that diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl index 2b4..de40040 100755 --- a/build-aux/bracket-spacing.pl +++ b/build-aux/bracket-spacing.pl @@ -1,6 +1,7 @@ #!/usr/bin/perl # # bracket-spacing.pl: Report any usage of 'function (..args..)' +# Also check for other syntax issues, such as correct use of ';' # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -31,6 +32,9 @@ foreach my $file (@ARGV) { while (defined (my $line = FILE)) { my $data = $line; +# Kill any quoted ; or +$data =~ s,'[;]','X',g; + # Kill any quoted strings $data =~ s,([^\\\]|\\.)*,XXX,g; @@ -125,6 +129,14 @@ foreach my $file (@ARGV) { $ret = 1; last; } + +# Require EOL, macro line continuation, or whitespace after :. +# Allow for (;;) as an exception. +while ($data =~ /;[^\\\n;)]/) { +print $file:$.: $line; +$ret = 1; +last; +} } close FILE; } diff --git a/cfg.mk b/cfg.mk index 55359e8..6e8b6d4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -845,7 +845,7 @@ syntax-check: $(top_srcdir)/HACKING bracket-spacing-check bracket-spacing-check: $(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \ $(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \ - (echo $(ME): incorrect whitespace around brackets, see HACKING for rules exit 1) + (echo $(ME): incorrect whitespace, see HACKING for rules exit 1) I see you changed this when pushing: - (echo $(ME): incorrect whitespace around brackets, see HACKING for rules exit 1) + { echo $(ME): incorrect whitespace, see HACKING for rules 2; \ + exit 1; } But it breaks build. I guess you indented to write 21 there. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't report error on successful media eject
On 05/28/2013 11:31 AM, Eric Blake wrote: On 05/28/2013 08:37 AM, Cole Robinson wrote: If we are just ejecting media, ret == -1 even after the retry loop determines that the tray is open, as requested. This means media disconnect always report's error. Fix it, and fix some other mini issues: - Don't overwrite the 'eject' error message if the retry loop fails - Move the retries decrement inside the loop, otherwise the final loop might succeed, yet retries == 0 and we will raise error - Setting ret = -1 in the disk-src check is unneeded - Fix comment typos cc: mpriv...@redhat.com --- src/qemu/qemu_hotplug.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) ACK. + +if (disk-src) { +/* deliberately don't depend on 'ret' as 'eject' may have failed the + * first time and we are gonna check the drive state anyway */ Comment motion, but might be worth s/gonna/going to/ while touching it (I'm actually surprised that thunderbird didn't flag the slang 'gonna' as a misspelled word). Thanks, pushed with that comment fix. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] syntax-check: mandate space after mid-line semicolon
On 05/28/2013 09:28 AM, Osier Yang wrote: On 25/05/13 01:19, Eric Blake wrote: Enforce the style cleanup in the previous patch. * build-aux/bracket-spacing.pl: Enforce trailing spacing. * cfg.mk (bracket-spacing-check): Tweak error wording. * docs/hacking.html.in: Document the rule. * HACKING: Regenerate. + (echo $(ME): incorrect whitespace, see HACKING for rules exit 1) I see you changed this when pushing: - (echo $(ME): incorrect whitespace around brackets, see HACKING for rules exit 1) + { echo $(ME): incorrect whitespace, see HACKING for rules 2; \ + exit 1; } But it breaks build. I guess you indented to write 21 there. Phooey. I was trying to copy how we did it elsewhere, and copied wrong without testing my final edit. I'll push the fix shortly. :( -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] syntax: fix broken error message in previous patch
Osier Yang pointed out that I introduced a syntax error in my syntax check (I really shouldn't make last-minute changes without testing them). /bin/sh: -c: line 2: syntax error near unexpected token `;' /bin/sh: -c: line 2: ` { echo 'maint.mk: incorrect whitespace, see HACKING for rules' 2; \' make: *** [bracket-spacing-check] Error 1 * cfg.mk (bracket-spacing-check): Fix copy-and-paste error. (sc_prohibit_duplicate_header): Use consistent spelling. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the build-breaker rule. cfg.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfg.mk b/cfg.mk index a01df56..4ffa020 100644 --- a/cfg.mk +++ b/cfg.mk @@ -779,7 +779,7 @@ sc_prohibit_duplicate_header: }' $$i || fail=1; \ done; \ if test $$fail -eq 1; then \ - { echo $(ME): avoid duplicate headers 2; exit 1; }\ + { echo '$(ME): avoid duplicate headers' 12; exit 1; } \ fi; # Don't include libvirt/*.h in form. @@ -845,7 +845,7 @@ syntax-check: $(top_srcdir)/HACKING bracket-spacing-check bracket-spacing-check: $(AM_V_GEN)files=`$(VC_LIST) | grep '\.c$$'`; \ $(PERL) $(top_srcdir)/build-aux/bracket-spacing.pl $$files || \ - { echo $(ME): incorrect whitespace, see HACKING for rules 2; \ + { echo '$(ME): incorrect whitespace, see HACKING for rules' 12; \ exit 1; } # sc_po_check can fail if generated files are not built first -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] conf: Generate address for scsi host device automatically
On 28/05/13 23:09, John Ferlan wrote: On 05/23/2013 12:48 PM, Osier Yang wrote: With unknown good reasons, the attribute bus of scsi device address is always set to 0, same for attribute target. (See virDomainDiskDefAssignAddress). Though we might need to change the algorithm to honor bus and target too, that's a different issue. The address generator for scsi host device in this patch just follows the unknown good reasons, only considering the controller and unit. It walks through all scsi controllers and their units, to see if the address $controller:0:0:$unit can be used, if found one, it sits on it, otherwise, it creates a new controller (actually the controller is implicitly created by someone else), and sits on $new_controller:0:0:0 instead. --- v1 - v2: * A new helper virDomainSCSIDriveAddressIsUsed * Move virDomainDefMaybeAddHostdevSCSIcontroller * More comments * Add testing. * problems fixed with the testing: 1) s/nscsi_controllers + 1/nscsi_controllers/, 2) Add the controller implicitly after a scsi hostdev def parsing, as it can use a new scsi controller index (nscsi_controllers), which should be added implicitly. First pass of code is found at: https://www.redhat.com/archives/libvir-list/2013-May/msg00340.html The previous/initial review and reply is found at: https://www.redhat.com/archives/libvir-list/2013-May/msg00506.html https://www.redhat.com/archives/libvir-list/2013-May/msg00526.html --- src/conf/domain_conf.c | 209 ++--- .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 95 ++ ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 106 +++ tests/qemuxml2xmltest.c| 2 + 4 files changed, 382 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-autogen-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-autogen-address.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ad5550c..86d743b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3782,6 +3782,148 @@ cleanup: return ret; } +/* Check if a drive type address $controller:0:0:$unit is already + * taken by a disk or not. + */ +static bool +virDomainDriveAddressIsUsedByDisk(virDomainDefPtr def, + enum virDomainDiskBus type, + unsigned int controller, + unsigned int unit) +{ +virDomainDiskDefPtr disk; +int i; + +for (i = 0; i def-ndisks; i++) { +disk = def-disks[i]; + +if (disk-bus != type || +disk-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) +continue; + +if (disk-info.addr.drive.controller == controller +disk-info.addr.drive.unit == unit +disk-info.addr.drive.bus == 0 +disk-info.addr.drive.target == 0) +return true; +} + +return false; +} + +/* Check if a drive type address $controller:0:0:$unit is already + * taken by a host device or not. + */ +static bool +virDomainDriveAddressIsUsedByHostdev(virDomainDefPtr def, + enum virDomainHostdevSubsysType type, + unsigned int controller, + unsigned int unit) +{ +virDomainHostdevDefPtr hostdev; +int i; + +for (i = 0; i def-nhostdevs; i++) { +hostdev = def-hostdevs[i]; + +if (hostdev-source.subsys.type != type) +continue; + +if (hostdev-info-addr.drive.controller == controller +hostdev-info-addr.drive.unit == unit +hostdev-info-addr.drive.bus == 0 +hostdev-info-addr.drive.target == 0) +return true; +} + +return false; +} + +static bool +virDomainSCSIDriveAddressIsUsed(virDomainDefPtr def, +unsigned int controller, +unsigned int unit) +{ +/* In current implementation, the maximum unit number of a controller + * is either 16 or 7 (narrow SCSI bus), and if the maximum unit number + * is 16, the controller itself is on unit 7 */ +if (unit == 7) FYI: The previous code cared that a wide address was being used when returning true - this code doesn't. Just making sure that change is intentional. Essentially, 7 cannot be used. The comments above the code talk about wide/narrow which it seems becomes irrelevant. The reality seems to be that you are avoiding using 7 altogether because it's reserved for the controller itself, correct? Regardless of whether a wide or narrow. The controller itself uses the unit number of the maximum value of the narrow band. At least that's how I'm reading it. You are right, will update the comment. +return true; + +if
[libvirt] [PATCH] iscsi: support IPv6 targets
Instead of resolving the host as IPv4 and only using the first result, resolve it as IPv6 as well and use the first address we've succesfully connected to as the portal name. --- src/libvirt_private.syms| 1 + src/storage/storage_backend_iscsi.c | 64 +-- src/util/virutil.c | 87 + src/util/virutil.h | 4 ++ 4 files changed, 102 insertions(+), 54 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9d5f74b..26c4553 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1931,6 +1931,7 @@ virEnumFromString; virEnumToString; virFindFCHostCapableVport; virFormatIntDecimal; +virGetConnectableIPAsString; virGetDeviceID; virGetDeviceUnprivSGIO; virGetFCHostNameByWWN; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index ad38ab2..f98fbce 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -1,7 +1,7 @@ /* * storage_backend_iscsi.c: storage backend for iSCSI handling * - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -44,58 +44,16 @@ #include vircommand.h #include virrandom.h #include virstring.h +#include virutil.h #define VIR_FROM_THIS VIR_FROM_STORAGE -static int -virStorageBackendISCSITargetIP(const char *hostname, - char *ipaddr, - size_t ipaddrlen) -{ -struct addrinfo hints; -struct addrinfo *result = NULL; -int ret; - -memset(hints, 0, sizeof(hints)); -hints.ai_flags = AI_ADDRCONFIG; -hints.ai_family = AF_INET; -hints.ai_socktype = SOCK_STREAM; -hints.ai_protocol = 0; - -ret = getaddrinfo(hostname, NULL, hints, result); -if (ret != 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(host lookup failed %s), - gai_strerror(ret)); -return -1; -} - -if (result == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(no IP address for target %s), - hostname); -return -1; -} - -if (getnameinfo(result-ai_addr, result-ai_addrlen, -ipaddr, ipaddrlen, NULL, 0, -NI_NUMERICHOST) 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(cannot format ip addr for %s), - hostname); -freeaddrinfo(result); -return -1; -} - -freeaddrinfo(result); -return 0; -} - static char * virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) { -char ipaddr[NI_MAXHOST]; -char *portal; +char *ip; +char *portal = NULL; +int port; if (source-nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, @@ -103,17 +61,15 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) return NULL; } -if (virStorageBackendISCSITargetIP(source-hosts[0].name, - ipaddr, sizeof(ipaddr)) 0) +port = source-hosts[0].port ? source-hosts[0].port : 3260; + +if (!(ip = virGetConnectableIPAsString(source-hosts[0].name, port))) return NULL; -if (virAsprintf(portal, %s:%d,1, ipaddr, -source-hosts[0].port ? -source-hosts[0].port : 3260) 0) { +if (virAsprintf(portal, %s,1, ip) 0) virReportOOMError(); -return NULL; -} +VIR_FREE(ip); return portal; } diff --git a/src/util/virutil.c b/src/util/virutil.c index 028f1d1..d8400db 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -650,6 +650,93 @@ cleanup: return result; } +/* + * Resolve 'host' and return the first address we've connected to + * as a string: + * address:port + * for IPv4 addresses and + * [address]:port + * for IPv6 addresses. + */ +char * +virGetConnectableIPAsString(const char *host, +int port) +{ +struct addrinfo hints; +struct addrinfo *res, *rp; +int saved_errno = EINVAL; +char *ret = NULL; +char *buf = NULL; +char *portstr = NULL; +int rc; + +if (VIR_ALLOC_N(buf, NI_MAXHOST) 0) +goto no_memory; + +if (virAsprintf(portstr, %d, port) 0) +goto no_memory; + +memset(hints, 0, sizeof(hints)); +hints.ai_socktype = SOCK_STREAM; +hints.ai_flags = AI_ADDRCONFIG; + +if ((rc = getaddrinfo(host, portstr, hints, res))) { +virReportError(VIR_ERR_UNKNOWN_HOST, + _(unable to resolve '%s': %s), + host, gai_strerror(rc)); +goto cleanup; +} + +for (rp = res; rp; rp = rp-ai_next) { +int sockfd; + +sockfd = socket(rp-ai_family, rp-ai_socktype,
Re: [libvirt] [PATCH v2] qemuOpenVhostNet: Decrease vhostfdSize on open failure
On 05/27/2013 02:44 AM, Michal Privoznik wrote: Currently, if there's an error opening /dev/vhost-net (e.g. because it doesn't exist) but it's not required we proceed with vhostfd array filled with -1 and vhostfdSize unchanged. Later, when constructing the qemu command line only non-negative items within vhostfd array are taken into account. This means, vhostfdSize may be greater than the actual count of non-negative items in vhostfd array. This results in improper command line arguments being generated, e.g.: -netdev tap,fd=21,id=hostnet0,vhost=on,vhostfd=(null) --- src/qemu/qemu_command.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0373626..c4a162a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -486,6 +486,10 @@ qemuOpenVhostNet(virDomainDefPtr def, but is unavailable)); goto error; } +VIR_WARN(Unable to open vhost-net. Opened so far %d, requested %d, + i, *vhostfdSize); +*vhostfdSize = i; +break; } } virDomainAuditNetDevice(def, net, /dev/vhost-net, *vhostfdSize); @@ -6560,12 +6564,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } for (i = 0; i vhostfdSize; i++) { -if (vhostfd[i] = 0) { -virCommandTransferFD(cmd, vhostfd[i]); -if (virAsprintf(vhostfdName[i], %d, vhostfd[i]) 0) { -virReportOOMError(); -goto cleanup; -} +virCommandTransferFD(cmd, vhostfd[i]); +if (virAsprintf(vhostfdName[i], %d, vhostfd[i]) 0) { +virReportOOMError(); +goto cleanup; } } ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Put the audio work on Windows 2008 Server 64bits
Hello, I am trying to put the audio audio work on Windows 2008 Server with 64bits but without success. I have tested with sound model ac97 and hda, and i always get the same error: the devices appears on Windows Device Manager with following message - Windows cannot load the device driver for this hardware. The driver may be corrupted or missing. (Code 39). I have qemu-kvm 1.2 and libvirt 0.10.2. I also tried different drivers with different sound model combinations but no success. Any suggestion of drivers that can i use and how can i get audio device working? Best Regards, -- Carlos Rodrigues c...@eurotux.com Engenheiro de Software Sénior Eurotux Informática, S.A. | www.eurotux.com (t) +351 253 680 300 signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] add hostdev passthrough common library
On 05/27/2013 12:52 AM, Marek Marczykowski wrote: On 16.05.2013 08:07, Chunyan Liu wrote: Write separate module for hostdev passthrough so that it could be used by all hypervisor drivers and maintain a global hostdev state. Signed-off-by: Chunyan Liu cy...@suse.com Some comments inline. Besides that seems to be working (with your next patch for libxl) :) --- po/POTFILES.in |1 + src/Makefile.am |1 + src/libvirt.c|5 + src/libvirt_private.syms | 15 + src/util/virhostdevmanager.c | 1218 ++ src/util/virhostdevmanager.h | 91 src/util/virpci.c| 17 +- src/util/virpci.h|7 +- src/util/virusb.c| 19 +- src/util/virusb.h|4 +- 10 files changed, 1362 insertions(+), 16 deletions(-) create mode 100644 src/util/virhostdevmanager.c create mode 100644 src/util/virhostdevmanager.h diff --git a/po/POTFILES.in b/po/POTFILES.in index f3ea4da..a7c21bf 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -188,6 +188,7 @@ src/util/viruri.c src/util/virusb.c src/util/virutil.c src/util/virxml.c +src/util/virhostdevmanager.c src/vbox/vbox_MSCOMGlue.c src/vbox/vbox_XPCOMCGlue.c src/vbox/vbox_driver.c diff --git a/src/Makefile.am b/src/Makefile.am index 4312c3c..a197c2b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -130,6 +130,7 @@ UTIL_SOURCES = \ util/virutil.c util/virutil.h \ util/viruuid.c util/viruuid.h \ util/virxml.c util/virxml.h \ +util/virhostdevmanager.c util/virhostdevmanager.h \ $(NULL) diff --git a/src/libvirt.c b/src/libvirt.c index 2b3515e..d9af5a6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -65,6 +65,7 @@ #include virthread.h #include virstring.h #include virutil.h +#include virhostdevmanager.h #ifdef WITH_TEST # include test/test_driver.h @@ -827,6 +828,7 @@ int virStateInitialize(bool privileged, if (virInitialize() 0) return -1; +virHostdevManagerInit(); for (i = 0 ; i virStateDriverTabCount ; i++) { if (virStateDriverTab[i]-stateInitialize) { VIR_DEBUG(Running global init for %s state driver, @@ -858,6 +860,9 @@ int virStateCleanup(void) { virStateDriverTab[i]-stateCleanup() 0) ret = -1; } + +virHostdevManagerCleanup(); + return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cdd0b41..824de4e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1991,6 +1991,21 @@ virXPathULong; virXPathULongHex; virXPathULongLong; +#util/virhostdevmanager.h +virHostdevManagerGetDefault; +virHostdevManagerInit; +virHostdevManagerCleanup; +virHostdevManagerPrepareHostdevs; +virHostdevManagerReAttachHostdevs; +virHostdevManagerPreparePciHostdevs; +virHostdevManagerPrepareUsbHostdevs; +virHostdevManagerReAttachPciHostdevs; +virHostdevManagerReAttachUsbHostdevs; +virGetActivePciHostdevs; +virGetActiveUsbHostdevs; +virGetDomainActivePciHostdevs; +virGetDomainActiveUsbHostdevs; +virFreeHostdevNameList; make check reports wrong symbol order here. # Let emacs know we want case-insensitive sorting # Local Variables: diff --git a/src/util/virhostdevmanager.c b/src/util/virhostdevmanager.c new file mode 100644 index 000..9034212 --- /dev/null +++ b/src/util/virhostdevmanager.c @@ -0,0 +1,1218 @@ +/* + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Chunyan Liu cy...@suse.com + */ +#include config.h + +#include virhostdevmanager.h + +#include sys/types.h +#include sys/stat.h +#include unistd.h +#include stdlib.h +#include stdio.h + +#include viralloc.h +#include virstring.h +#include virfile.h +#include virerror.h +#include virlog.h +#include virpci.h +#include virusb.h +#include virnetdev.h + +/* For virReportOOMError() and virReportSystemError() */ +#define VIR_FROM_THIS VIR_FROM_NONE + +struct _virHostdevManager{ +char *stateDir; + +
Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.
On 05/28/2013 03:59 AM, Guannan Ren wrote: For qemu, if the -smp N or the value of maxcpus is given, it define the number of vcpu of guest whenever the vcpu topology is defined or not. But if the -smp N and maxcpus are missing, the topology can compute and define vcpus for guest automatically by math: vcpu number = sockets*cores*threads For libvirt, as vcpu is always mandatory, so we can ask topology to match maximum vcpu numbers. --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..ffdc6da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (def-cpu-sockets -def-maxvcpus +def-maxvcpus != def-cpu-sockets * def-cpu-cores * def-cpu-threads) { virReportError(VIR_ERR_XML_DETAIL, %s, - _(Maximum CPUs greater than topology limit)); + _(Topology limit does not match maximum CPUs)); Is this going to reject XML that was previously accepted? Is there a bugzilla showing what happens if this patch is not incorporated? I'm worried about introducing an unintentional regression if we include this in 1.0.6 without more justification. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Put the audio work on Windows 2008 Server 64bits
On 05/28/2013 11:06 AM, Carlos Rodrigues wrote: Hello, I am trying to put the audio audio work on Windows 2008 Server with 64bits but without success. I have tested with sound model ac97 and hda, and i always get the same error: the devices appears on Windows Device Manager with following message - Windows cannot load the device driver for this hardware. The driver may be corrupted or missing. (Code 39). I have qemu-kvm 1.2 and libvirt 0.10.2. I also tried different drivers with different sound model combinations but no success. Any suggestion of drivers that can i use and how can i get audio device working? Have you considered using SPICE for your audio? Personally, I haven't tried audio passthrough to my one windows guest, so I don't have many ideas; it may be better to ask on spice lists, precisely because that's where you will find more developers that actually play with that sort of thing. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virsh: migrate: Don't disallow --p2p and --migrateuri
Because it's a valid combination. p2p still uses a separate channel for qemu migration, so there's value in letting the user specify a manual migrate URI for overriding auto-port, or libvirt's FQDN lookup. What _isn't_ allowed is --migrateuri and TUNNELLED, since there is no separate migration channel. Disallow that instead --- tools/virsh-domain.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4fdf4ba..eb8688d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8306,15 +8306,15 @@ doMigrate(void *opaque) if ((flags VIR_MIGRATE_PEER2PEER) || vshCommandOptBool(cmd, direct)) { -/* For peer2peer migration or direct migration we only expect one URI - * a libvirt URI, or a hypervisor specific URI. */ -if (migrateuri != NULL) { +/* migrateuri doesn't make sense for tunnelled migration */ +if (flags VIR_MIGRATE_TUNNELLED migrateuri != NULL) { vshError(ctl, %s, _(migrate: Unexpected migrateuri for peer2peer/direct migration)); goto out; } -if (virDomainMigrateToURI2(dom, desturi, NULL, xml, flags, dname, 0) == 0) +if (virDomainMigrateToURI2(dom, desturi, migrateuri, + xml, flags, dname, 0) == 0) ret = '0'; } else { /* For traditional live migration, connect to the destination host directly. */ -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: migration: Improve p2p error if we can't open conn
By actually showing the Open() error to the user --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 19b1236..9ac9be4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3472,7 +3472,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, qemuDomainObjExitRemote(vm); if (dconn == NULL) { virReportError(VIR_ERR_OPERATION_FAILED, - _(Failed to connect to remote libvirt URI %s), dconnuri); + _(Failed to connect to remote libvirt URI %s: %s), + dconnuri, virGetLastErrorMessage()); virObjectUnref(cfg); return -1; } -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: migration: error if tunnelled + storage specified
Since as the code indicates it doesn't work yet, so let's be explicit about it. --- src/qemu/qemu_migration.c | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 9ac9be4..ffc86a4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1900,12 +1900,13 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver, if (flags (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NBD_SERVER)) { /* TODO support NBD for TUNNELLED migration */ -if (flags VIR_MIGRATE_TUNNELLED) -VIR_DEBUG(NBD in tunnelled migration is currently not supported); -else { -cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; -priv-nbdPort = 0; +if (flags VIR_MIGRATE_TUNNELLED) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s, +_(NBD in tunnelled migration is currently not supported)); +goto cleanup; } +cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; +priv-nbdPort = 0; } if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0))) @@ -2200,16 +2201,11 @@ done: if (mig-nbd flags (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NBD_SERVER)) { -/* TODO support NBD for TUNNELLED migration */ -if (flags VIR_MIGRATE_TUNNELLED) -VIR_DEBUG(NBD in tunnelled migration is currently not supported); -else { -if (qemuMigrationStartNBDServer(driver, vm, listenAddr) 0) { -/* error already reported */ -goto endjob; -} -cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; +if (qemuMigrationStartNBDServer(driver, vm, listenAddr) 0) { +/* error already reported */ +goto endjob; } +cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: migration: Improve p2p error if we can't open conn
On 05/28/2013 02:44 PM, Cole Robinson wrote: By actually showing the Open() error to the user --- src/qemu/qemu_migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Does qemuDomainObjExitRemote(vm) risk clobbering the last error message? Since it potentially unrefs vm on the last reference, I'm wondering if we have to cache the last error at the right point in time instead of relying on current behavior. But obviously it worked in your testing as written. diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 19b1236..9ac9be4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3472,7 +3472,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver, qemuDomainObjExitRemote(vm); if (dconn == NULL) { virReportError(VIR_ERR_OPERATION_FAILED, - _(Failed to connect to remote libvirt URI %s), dconnuri); + _(Failed to connect to remote libvirt URI %s: %s), + dconnuri, virGetLastErrorMessage()); virObjectUnref(cfg); return -1; Another alternative is to just return -1 directly instead of doing a virReportError() that overwrites the original error, although I'm not sure if that loses some context that would help the user. Can you include a sample error message in your commit log that shows the improvement? If so, I'm inclined to ACK this. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: migration: error if tunnelled + storage specified
On 05/28/2013 02:44 PM, Cole Robinson wrote: Since as the code indicates it doesn't work yet, so let's be explicit about it. --- src/qemu/qemu_migration.c | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) ACK. An explicit error is always nicer than a nebulous todo that plows on in unsupported state. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh: migrate: Don't disallow --p2p and --migrateuri
On 05/28/2013 02:44 PM, Cole Robinson wrote: Because it's a valid combination. p2p still uses a separate channel for qemu migration, so there's value in letting the user specify a manual migrate URI for overriding auto-port, or libvirt's FQDN lookup. What _isn't_ allowed is --migrateuri and TUNNELLED, since there is no separate migration channel. Disallow that instead --- tools/virsh-domain.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) ACK. This exposes more power of the underlying API (the fact that it only touches tools/virsh-domain.c is good). diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4fdf4ba..eb8688d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8306,15 +8306,15 @@ doMigrate(void *opaque) if ((flags VIR_MIGRATE_PEER2PEER) || vshCommandOptBool(cmd, direct)) { -/* For peer2peer migration or direct migration we only expect one URI - * a libvirt URI, or a hypervisor specific URI. */ -if (migrateuri != NULL) { +/* migrateuri doesn't make sense for tunnelled migration */ +if (flags VIR_MIGRATE_TUNNELLED migrateuri != NULL) { vshError(ctl, %s, _(migrate: Unexpected migrateuri for peer2peer/direct migration)); goto out; } -if (virDomainMigrateToURI2(dom, desturi, NULL, xml, flags, dname, 0) == 0) +if (virDomainMigrateToURI2(dom, desturi, migrateuri, + xml, flags, dname, 0) == 0) ret = '0'; } else { /* For traditional live migration, connect to the destination host directly. */ -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2] Expose all CPU features in host definition
[V2 - Improve the error handling] I've opened BZ 697141 on this as I would consider it more a bug than a feature request. Anyway, to re-iterate my rationale from the BZ: The virConnectGetCapabilities API describes the host capabilities by returning an XML description that includes the CPU model name and a set of CPU features. The problem is that any features that are part of the CPU model are not explicitly listed, they are assumed to be part of the definition of that CPU model. This makes it extremely difficult for the caller of this API to check for the presence of a specific CPU feature, the caller would have to know what features are part of which CPU models, a very daunting task. This patch solves this problem by having the API return a model name, as it currently does, but it will also explicitly list all of the CPU features that are present. This would make it much easier for a caller of this API to check for specific features. Signed-off-by: Don Dugger donald.d.dug...@intel.com -- src/cpu/cpu_x86.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 5d479c2..098ab05 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1296,6 +1296,35 @@ x86GuestData(virCPUDefPtr host, return x86Compute(host, guest, data, message); } +static int +x86AddFeatures(virCPUDefPtr cpu, + struct x86_map *map) +{ +int ret; +const struct x86_model *candidate; +const struct x86_feature *feature = map-features; + +candidate = map-models; +while (candidate != NULL) { +if (STREQ(cpu-model, candidate-name)) +break; +candidate = candidate-next; +} +if (!candidate) { +virReportError(VIR_ERR_INTERNAL_ERROR, + Odd, %s not a known CPU model\n, cpu-model); +return -1; +} +while (feature != NULL) { +if (x86DataIsSubset(candidate-data, feature-data)) { +if ((ret = virCPUDefAddFeature(cpu, feature-name, VIR_CPU_FEATURE_DISABLE)) 0) +return ret; +} +feature = feature-next; +} +return 0; +} + static int x86Decode(virCPUDefPtr cpu, @@ -1383,6 +1412,8 @@ x86Decode(virCPUDefPtr cpu, goto out; } +if (x86AddFeatures(cpuModel, map) 0) +goto out; cpu-model = cpuModel-model; cpu-vendor = cpuModel-vendor; cpu-nfeatures = cpuModel-nfeatures; -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device
On 05/24/2013 04:46 PM, Eric Blake wrote: From: Seiji Aguchi seiji.agu...@hds.com [Problem] Currently, guest OS's messages can be logged to a local disk of host OS by creating chadevs with options below. -chardev file,id=charserial0,path=log file's path -device isa-serial,chardev=chardevserial0,id=serial0 Seiji's patch plus my initial review comments; I've ack'd this much, but want to add a round-trip xml2xml test before pushing anything (Seiji, if you want to use this as a starting point, rather than waiting for my long weekend, feel free to post a v3). Sorry, but this needs a v3, and I'm going to hand it back to you to fix. I tried adding a testsuite, but it proved that you weren't outputting the startupPolicy during dumpxml formatting. My initial attempt to add it backfired (I'll post it below); I'm worried that we are playing too fast and loose with a union type, since even with my patch in place, 'make check' fails with problems like: 84) QEMU XML-2-XML serial-dev ... Offset 913 Expect [/ target port='0'/ /serial console type='dev' source path='/dev/ttyS2] Actual [ startupPolicy='(null)'/ target port='0'/ /serial console type='dev' source path='/dev/ttyS2' startupPolicy='(null)] which is a bug, since you said startupPolicy should only be tied to files and not other serial types. Do we need to repeat the startupPolicy on the console mirror of the first serial device, or is that unnecessary back-compat, and where we can safely declare that startupPolicy is only emitted for the serial side of things? All told, your patch is not complete until it passes 'make check' with a new xml2xml test that proves we can round trip the new policy. diff --git c/src/conf/domain_conf.c w/src/conf/domain_conf.c index c24a9f0..5af5e40 100644 --- c/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -14547,8 +14547,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def-type != VIR_DOMAIN_CHR_TYPE_PTY || (def-data.file.path !(flags VIR_DOMAIN_XML_INACTIVE))) { -virBufferEscapeString(buf, source path='%s'/\n, +virBufferEscapeString(buf, source path='%s', def-data.file.path); + +if (def-data.file.path def-data.file.startupPolicy) { +const char *policy = virDomainStartupPolicyTypeToString(def-data.file.startupPolicy); +virBufferAsprintf(buf, startupPolicy='%s', policy); +} +virBufferAddLit(buf, /\n); } break; diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args new file mode 100644 index 000..24977f2 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi -boot c -usb -hdc /tmp/idedisk.img \ +-chardev file,id=charserial0,path=/tmp/serial.log \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml new file mode 100644 index 000..34e0eb9 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml @@ -0,0 +1,35 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + source file='/tmp/idedisk.img'/ + target dev='hdc' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='2'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +serial type='file' + source path='/tmp/serial.log' startupPolicy='optional'/ + target port='0'/ +/serial +console type='file' + source path='/tmp/serial.log'/ + target type='serial' port='0'/ +/console +memballoon model='virtio'/ + /devices +/domain diff --git c/tests/qemuxml2argvtest.c w/tests/qemuxml2argvtest.c index 2c7fd01..22f4782 100644 --- c/tests/qemuxml2argvtest.c +++ w/tests/qemuxml2argvtest.c @@ -727,6 +727,8 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE,
[libvirt] [PATCH] build: drop unused variable
Compilation for mingw failed: ../../src/util/virutil.c: In function 'virGetWin32DirectoryRoot': ../../src/util/virutil.c:1094:9: error: unused variable 'ret' [-Werror=unused-variable] * src/util/virutil.c (virGetWin32DirectoryRoot): Silence compiler warning. Signed-off-by: Eric Blake ebl...@redhat.com --- Compilation still fails in the tests directory, due to the use of fprintf(stderr, %zu, size_t); I'm still debating whether I can get it fixed in gnulib and take the risk of a gnulib backport before rc2, or whether we'll have to come up with some other less-invasive workaround. src/util/virutil.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 028f1d1..a29da14 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1091,7 +1091,6 @@ static int virGetWin32DirectoryRoot(char **path) { char windowsdir[MAX_PATH]; -int ret = 0; *path = NULL; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: drop unused variable
On 05/28/2013 05:20 PM, Eric Blake wrote: Compilation for mingw failed: ../../src/util/virutil.c: In function 'virGetWin32DirectoryRoot': ../../src/util/virutil.c:1094:9: error: unused variable 'ret' [-Werror=unused-variable] * src/util/virutil.c (virGetWin32DirectoryRoot): Silence compiler warning. Signed-off-by: Eric Blake ebl...@redhat.com --- I pushed this under the build-breaker rule, by the way. Compilation still fails in the tests directory, due to the use of fprintf(stderr, %zu, size_t); I'm still debating whether I can get it fixed in gnulib and take the risk of a gnulib backport before rc2, or whether we'll have to come up with some other less-invasive workaround. Dan already posted a potential workaround, but he put it on hold pending whatever I can determine on the gnulib side of things. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix build with older gcc
gcc 4.1.2 (hello, RHEL 5!) fails to build on 32-bit platforms with: conf/domain_conf.c: In function 'virDomainDefParseXML': conf/domain_conf.c:10581: warning: integer constant is too large for 'long' type * src/conf/domain_conf.c (virDomainDefParseXML): Mark large constants. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..2b4e160 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10578,7 +10578,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (def-cputune.quota 0 (def-cputune.quota 1000 || - def-cputune.quota 18446744073709551)) { + def-cputune.quota 18446744073709551LL)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Value of cputune quota must be in range [1000, 18446744073709551])); @@ -10610,7 +10610,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (def-cputune.emulator_quota 0 (def-cputune.emulator_quota 1000 || - def-cputune.emulator_quota 18446744073709551)) { + def-cputune.emulator_quota 18446744073709551LL)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Value of cputune emulator_quota must be in range [1000, 18446744073709551])); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix build with older gcc
On 05/28/2013 05:33 PM, Eric Blake wrote: gcc 4.1.2 (hello, RHEL 5!) fails to build on 32-bit platforms with: conf/domain_conf.c: In function 'virDomainDefParseXML': conf/domain_conf.c:10581: warning: integer constant is too large for 'long' type * src/conf/domain_conf.c (virDomainDefParseXML): Mark large constants. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Introduced in v1.0.4-6-gf8e3221; pushed under build-breaker rule. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device
Eric, -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Eric Blake Sent: Tuesday, May 28, 2013 7:08 PM Cc: libvir-list@redhat.com; dle-deve...@lists.sourceforge.net; Tomoki Sekiyama Subject: Re: [libvirt] [PATCHv2] xml: introduce startupPolicy for chardev device On 05/24/2013 04:46 PM, Eric Blake wrote: From: Seiji Aguchi seiji.agu...@hds.com [Problem] Currently, guest OS's messages can be logged to a local disk of host OS by creating chadevs with options below. -chardev file,id=charserial0,path=log file's path -device isa-serial,chardev=chardevserial0,id=serial0 Seiji's patch plus my initial review comments; I've ack'd this much, but want to add a round-trip xml2xml test before pushing anything (Seiji, if you want to use this as a starting point, rather than waiting for my long weekend, feel free to post a v3). Sorry, but this needs a v3, and I'm going to hand it back to you to fix. I tried adding a testsuite, but it proved that you weren't outputting the startupPolicy during dumpxml formatting. My initial attempt to add it backfired (I'll post it below); Thank you for handling/testing my patch. I'm worried that we are playing too fast and loose with a union type, since even with my patch in place, 'make check' fails with problems like: 84) QEMU XML-2-XML serial-dev ... Offset 913 Expect [/ target port='0'/ /serial console type='dev' source path='/dev/ttyS2] Actual [ startupPolicy='(null)'/ target port='0'/ /serial console type='dev' source path='/dev/ttyS2' startupPolicy='(null)] which is a bug, since you said startupPolicy should only be tied to files and not other serial types. Do we need to repeat the startupPolicy on the console mirror of the first serial device, or is that unnecessary back-compat, and where we can safely declare that startupPolicy is only emitted for the serial side of things? I need to think carefully about it before deciding it in a hurry. All told, your patch is not complete until it passes 'make check' with a new xml2xml test that proves we can round trip the new policy. Anyway, I will post the v3 patch by fixing the issue. Seiji diff --git c/src/conf/domain_conf.c w/src/conf/domain_conf.c index c24a9f0..5af5e40 100644 --- c/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -14547,8 +14547,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def-type != VIR_DOMAIN_CHR_TYPE_PTY || (def-data.file.path !(flags VIR_DOMAIN_XML_INACTIVE))) { -virBufferEscapeString(buf, source path='%s'/\n, +virBufferEscapeString(buf, source path='%s', def-data.file.path); + +if (def-data.file.path def-data.file.startupPolicy) { +const char *policy = virDomainStartupPolicyTypeToString(def-data.file.startupPolicy); +virBufferAsprintf(buf, startupPolicy='%s', policy); +} +virBufferAddLit(buf, /\n); } break; diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args new file mode 100644 index 000..24977f2 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi -boot c -usb -hdc /tmp/idedisk.img \ +-chardev file,id=charserial0,path=/tmp/serial.log \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git c/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml new file mode 100644 index 000..34e0eb9 --- /dev/null +++ w/tests/qemuxml2argvdata/qemuxml2argv-serial-source-optional.xml @@ -0,0 +1,35 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219136/memory + currentMemory unit='KiB'219136/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='file' device='disk' + source file='/tmp/idedisk.img'/ + target dev='hdc' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='2'/ +/disk +controller type='usb' index='0'/ +
Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.
On 05/29/2013 04:20 AM, Eric Blake wrote: On 05/28/2013 03:59 AM, Guannan Ren wrote: For qemu, if the -smp N or the value of maxcpus is given, it define the number of vcpu of guest whenever the vcpu topology is defined or not. But if the -smp N and maxcpus are missing, the topology can compute and define vcpus for guest automatically by math: vcpu number = sockets*cores*threads For libvirt, as vcpu is always mandatory, so we can ask topology to match maximum vcpu numbers. --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9656af..ffdc6da 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11815,10 +11815,10 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (def-cpu-sockets -def-maxvcpus +def-maxvcpus != def-cpu-sockets * def-cpu-cores * def-cpu-threads) { virReportError(VIR_ERR_XML_DETAIL, %s, - _(Maximum CPUs greater than topology limit)); + _(Topology limit does not match maximum CPUs)); Is this going to reject XML that was previously accepted? Is there a bugzilla showing what happens if this patch is not incorporated? I'm worried about introducing an unintentional regression if we include this in 1.0.6 without more justification. Yes, it make the cpu topology limit setting more strict. Alternatively, we can add more comments for topology to make it clear that vcpu decide the number of vcpus, the sockets*cores*threads had better be equal to vcpu numbers. The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017 Description of problem: libvirt should check if vcpu topology is right. If the wrong vcpu topology is given in xml , the wrong arguments also be passed to qemu-kvm. vcpu number = sockets*cores*threads Steps to Reproduce: 1.# virsh start vm Domain vm started 2.# virsh dumpxml vm domain type='kvm' id='104' ... vcpu placement='static'4/vcpu .. cpu topology sockets='1' cores='4' threads='2'/ /cpu 3.# ps -ef|grep qemu-kvm qemu 21296 1 14 16:41 ?00:00:17 /usr/libexec/qemu-kvm -name vm -S -M rhel6.4.0 -enable-kvm -m 1024 -smp 4,sockets=1,cores=4,threads=2 .. Actual results: Wrong vcpu topology can be given in xml and passed to qemu-kvm Expected results: libvirt should check if vcpu topology is right. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus
--- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..27c1580 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -941,7 +941,9 @@ virtual CPU provided to the guest. Three non-zero values have to be given for codesockets/code, codecores/code, and codethreads/code: total number of CPU sockets, number of cores per -socket, and number of threads per core, respectively./dd +socket, and number of threads per core, respectively. The maximum number +of virtual CPUs is defined by codevcpu/code element, the product of +the three valuses should be equal to the maximum vcpus./dd dtcodefeature/code/dt ddThe codecpu/code element can contain zero or more -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: match vcpu topology to maximum vcpu numbers.
On 05/28/2013 07:32 PM, Guannan Ren wrote: if (def-cpu-sockets -def-maxvcpus +def-maxvcpus != def-cpu-sockets * def-cpu-cores * def-cpu-threads) { virReportError(VIR_ERR_XML_DETAIL, %s, - _(Maximum CPUs greater than topology limit)); + _(Topology limit does not match maximum CPUs)); Is this going to reject XML that was previously accepted? Is there a bugzilla showing what happens if this patch is not incorporated? I'm worried about introducing an unintentional regression if we include this in 1.0.6 without more justification. Yes, it make the cpu topology limit setting more strict. I'm okay with making things more strict, if it makes sense. But isn't it feasible to support a topology that has more slots than the maximum? That is, since a bare metal machine can have two sockets but only populate one, why can't a virtual machine have 2 sockets but set a max vcpu of 1 so that only one socket can be populated? Does qemu itself enforce that there be a match? That's why I'm worried about enforcing that the user can't have a mismatch between maxvcpus vs. topology, if they specify both. I'm also worried that this check might fire if the user specifies only topology or only maxvcpus; I want to make sure that we only do the sanity checking if both numbers are provided. Alternatively, we can add more comments for topology to make it clear that vcpu decide the number of vcpus, the sockets*cores*threads had better be equal to vcpu numbers. If there is a problem with a mismatch, then we should document that a match is required, and fail on mismatch. Merely documenting that a match is required without enforcing it is too weak; and enforcing it without documentation is a disservice. I haven't ruled out enforcing a match, but want to make sure that we aren't artificially crippling something that might prove useful in reality, when compared with what bare-metal setups can provide. The bz:https://bugzilla.redhat.com/show_bug.cgi?id=880017 Description of problem: libvirt should check if vcpu topology is right. If the wrong vcpu topology is given in xml , the wrong arguments also be passed to qemu-kvm. vcpu number = sockets*cores*threads Steps to Reproduce: 1.# virsh start vm Domain vm started 2.# virsh dumpxml vm domain type='kvm' id='104' ... vcpu placement='static'4/vcpu .. cpu topology sockets='1' cores='4' threads='2'/ /cpu 3.# ps -ef|grep qemu-kvm qemu 21296 1 14 16:41 ?00:00:17 /usr/libexec/qemu-kvm -name vm -S -M rhel6.4.0 -enable-kvm -m 1024 -smp 4,sockets=1,cores=4,threads=2 .. Actual results: Wrong vcpu topology can be given in xml and passed to qemu-kvm Expected results: libvirt should check if vcpu topology is right. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] doc: cpu topology values should be equal to maxmium vcpus
On 05/28/2013 08:53 PM, Guannan Ren wrote: --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..27c1580 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -941,7 +941,9 @@ virtual CPU provided to the guest. Three non-zero values have to be given for codesockets/code, codecores/code, and codethreads/code: total number of CPU sockets, number of cores per -socket, and number of threads per core, respectively./dd +socket, and number of threads per core, respectively. The maximum number +of virtual CPUs is defined by codevcpu/code element, the product of +the three valuses should be equal to the maximum vcpus./dd s/valuses/values/ Again, if we are going to document that they must be equal, then the code should match; otherwise, if we are going to allow a difference between the two, then the wording should be a bit nicer (It is recommended that if both maxvcpu and topology are provided, then the two numbers should match). Or maybe a one-sided restriction makes sense, where maxvcpus can be less than or equal, but not greater than, an explicit topology. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: fix build with newer gnutls
Building with gnutls 3.2.0 (such as shipped with current cygwin) fails with: rpc/virnettlscontext.c: In function 'virNetTLSSessionGetKeySize': rpc/virnettlscontext.c:1358:5: error: implicit declaration of function 'gnutls_cipher_get_key_size' [-Wimplicit-function-declaration] Yeah, it's stupid that gnutls broke API by moving their declaration into a new header without including that header from the old one, but it's easy enough to work around, all without breaking on gnutls 1.4.1 (hello RHEL 5) that lacked the new header. * src/rpc/virnettlscontext.c (includes): Include additional header. * configure.ac (gnutls): Check for gnutls/crypto.h. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the build-breaker rule, once my testing completes across multiple systems with various gnutls versions (1.4.1 in RHEL5, 2.12.23 in Fedora 18, 3.2.0 in Cygwin). configure.ac | 1 + src/rpc/virnettlscontext.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index 7041710..0b3d028 100644 --- a/configure.ac +++ b/configure.ac @@ -1087,6 +1087,7 @@ if test x$with_gnutls != xno; then if test $GNUTLS_FOUND = no; then fail=0 AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1]) +AC_CHECK_HEADERS([gnutls/crypto.h]) AC_CHECK_LIB([gnutls], [gnutls_handshake],[], [fail=1], [-lgcrypt]) test $fail = 0 GNUTLS_FOUND=yes diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index e29c439..f2ac551 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -25,6 +25,9 @@ #include stdlib.h #include gnutls/gnutls.h +#if HAVE_GNULTLS_CRYPTO_H +# include gnutls/crypto.h +#endif #include gnutls/x509.h #include gnutls_1_0_compat.h -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix build with newer gnutls
On 05/28/2013 09:23 PM, Eric Blake wrote: Building with gnutls 3.2.0 (such as shipped with current cygwin) fails with: rpc/virnettlscontext.c: In function 'virNetTLSSessionGetKeySize': rpc/virnettlscontext.c:1358:5: error: implicit declaration of function 'gnutls_cipher_get_key_size' [-Wimplicit-function-declaration] Yeah, it's stupid that gnutls broke API by moving their declaration into a new header without including that header from the old one, but it's easy enough to work around, all without breaking on gnutls 1.4.1 (hello RHEL 5) that lacked the new header. * src/rpc/virnettlscontext.c (includes): Include additional header. * configure.ac (gnutls): Check for gnutls/crypto.h. Signed-off-by: Eric Blake ebl...@redhat.com --- Pushing under the build-breaker rule, once my testing completes across multiple systems with various gnutls versions (1.4.1 in RHEL5, 2.12.23 in Fedora 18, 3.2.0 in Cygwin). Or not - it failed my builds. +++ b/configure.ac @@ -1087,6 +1087,7 @@ if test x$with_gnutls != xno; then if test $GNUTLS_FOUND = no; then fail=0 AC_CHECK_HEADER([gnutls/gnutls.h], [], [fail=1]) +AC_CHECK_HEADERS([gnutls/crypto.h]) This line needs tweaking to pre-include gnutls/gnutls.h before probing for crypto.h. I'll post a v2 once I complete more testing. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list