On Fri, Apr 10, 2015 at 17:36:23 -0400, John Ferlan wrote:
> Rather than use the default numbering scheme of 1..number of iothreads
> defined for the domain, use the iothreadid's list for the iothread_id and
> possibly augmenting the alias using the iothreadsid name.
>
> This also requires adjusting the iothreadpids structure to include room for
> the iothread_id and name (if found in the alias, thus iothreadids entry).
>
> The iothreadpids will keep track of the live system, while iothreadids will
> be used for the configuration.
>
> Now that the iothreadpids list keeps track of the iothread_id's, these
> can be used in place of the many places where a for loop would "know"
> that the ID was "+ 1" from the array element.
>
> The new tests ensure usage of the <iothreadid> values for an exact number
> of iothreads, the usage of a smaller number of <iothreadid> values than
> iothreads that exist (and usage of the default numbering scheme), and the
> usage of the optional name value to provide the alias for the IOThread.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/qemu/qemu_cgroup.c | 13 ++-
> src/qemu/qemu_command.c | 104 ++++++++++++++++++---
> src/qemu/qemu_command.h | 4 +
> src/qemu/qemu_domain.c | 40 +++++++-
> src/qemu/qemu_domain.h | 2 +
> src/qemu/qemu_driver.c | 35 ++++---
> src/qemu/qemu_process.c | 29 +++++-
> .../qemuxml2argv-iothreads-ids-partial.args | 10 ++
> .../qemuxml2argv-iothreads-ids-partial.xml | 33 +++++++
> .../qemuxml2argv-iothreads-ids.args | 8 ++
> .../qemuxml2argv-iothreads-ids.xml | 33 +++++++
> .../qemuxml2argv-iothreads-name.args | 17 ++++
> .../qemuxml2argv-iothreads-name.xml | 44 +++++++++
> tests/qemuxml2argvtest.c | 4 +
> tests/qemuxml2xmltest.c | 3 +
> 15 files changed, 339 insertions(+), 40 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml
>
...
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e7e0937..68c85e2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -678,6 +678,57 @@ qemuOpenVhostNet(virDomainDefPtr def,
> }
>
> int
> +qemuDomainParseIOThreadAlias(char *alias,
> + unsigned int *iothread_id,
> + char **name)
> +{
> + unsigned int idval;
> + char *idname = NULL;
> +
> + /* IOThread's alias is either "iothread#" or "name_iothread#", where
I don't really think we should put any user-configurable stuff into the
alias. We can keep the name internally and use it for lookup but using
it in the alias can be tricky.
If it would be part of the alias, the name definitely should not start
with the user configurable part.
> + * name is a user definable prefix for the alias and the # is the
> + * iothreadids iothread_id provided in XML or generated during
> + * post parse processing
> + */
> + if (STRPREFIX(alias, "iothread")) {
> + if (virStrToLong_ui(alias + strlen("iothread"),
> + NULL, 10, &idval) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to find iothread id for '%s'"),
> + alias);
> + return -1;
> + }
> + /* Default - no need to do anything with name */
> + } else {
> + char *spot = strstr(alias, "_iothread");
> +
> + if (!spot) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Malformed IOThreads alias '%s'"),
> + alias);
> + return -1;
> + }
> +
> + /* Pick off the user defined name from the front */
> + if (VIR_STRNDUP(idname, alias, spot - alias) < 0)
> + return -1;
> +
> + if (virStrToLong_ui(alias + strlen(idname) + strlen("_iothread"),
> + NULL, 10, &idval) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to find iothread id for '%s'"),
> + alias);
> + VIR_FREE(idname);
> + return -1;
> + }
> + }
> +
> + *iothread_id = idval;
> + *name = idname;
> + return 0;
> +}
> +
> +int
> qemuNetworkPrepareDevices(virDomainDefPtr def)
> {
> int ret = -1;
> @@ -3985,11 +4036,11 @@ qemuCheckIOThreads(virDomainDefPtr def,
> return false;
> }
>
> - /* Value larger than iothreads available? */
> - if (disk->iothread > def->iothreads) {
> + /* Can we find the disk iothread in the iothreadid list? */
Comment is not necessary.
> + if (!virDomainIOThreadIDFind(def, disk->iothread)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Disk iothread '%u' invalid only %u IOThreads"),
> - disk->iothread, def->iothreads);
> + _("Disk iothread '%u' not defined"),
> + disk->iothread);
> return false;
> }
>
> @@ -4005,6 +4056,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
> {
> virBuffer opt = VIR_BUFFER_INITIALIZER;
> const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
> + virDomainIOThreadIDDefPtr iothrid = NULL;
> + char *disk_iothr_alias = NULL;
> int controllerModel;
>
> if (virDomainDiskDefDstDuplicates(def))
> @@ -4194,10 +4247,27 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
> }
> break;
> case VIR_DOMAIN_DISK_BUS_VIRTIO:
> + if (disk->iothread) {
> + if (!(iothrid = virDomainIOThreadIDFind(def, disk->iothread))) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("cannot find iothread '%u' in iothreadids"),
> + disk->iothread);
> + goto error;
> + }
> + if (iothrid->name) {
> + if (virAsprintf(&disk_iothr_alias, "%s_iothread%u",
> + iothrid->name, iothrid->iothread_id) < 0)
As said, we should not put the strings into qemu... or definitely not at
the beginning.
> + goto error;
> + } else {
> + if (virAsprintf(&disk_iothr_alias, "iothread%u",
> + iothrid->iothread_id) < 0)
> + goto error;
> + }
> + }
> if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> virBufferAddLit(&opt, "virtio-blk-ccw");
> - if (disk->iothread)
> - virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread);
> + if (disk_iothr_alias)
> + virBufferAsprintf(&opt, ",iothread=%s", disk_iothr_alias);
> } else if (disk->info.type ==
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
> virBufferAddLit(&opt, "virtio-blk-s390");
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list