From nobody Thu May 15 21:38:00 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1508873778311631.2174826529969; Tue, 24 Oct 2017 12:36:18 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A5C11CD262; Tue, 24 Oct 2017 19:36:16 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8545A18006; Tue, 24 Oct 2017 19:36:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 196D76EF2D; Tue, 24 Oct 2017 19:36:11 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v9OJZlic009740 for ; Tue, 24 Oct 2017 15:35:47 -0400 Received: by smtp.corp.redhat.com (Postfix) id 563295D6AE; Tue, 24 Oct 2017 19:35:47 +0000 (UTC) Received: from mx1.redhat.com (ext-mx03.extmail.prod.ext.phx2.redhat.com [10.5.110.27]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4E9FF5D6A6 for ; Tue, 24 Oct 2017 19:35:45 +0000 (UTC) Received: from mail-qt0-f179.google.com (mail-qt0-f179.google.com [209.85.216.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A46776A7DF for ; Tue, 24 Oct 2017 19:35:43 +0000 (UTC) Received: by mail-qt0-f179.google.com with SMTP id k31so31967997qta.6 for ; Tue, 24 Oct 2017 12:35:43 -0700 (PDT) Received: from dawid-fedora.datto.lan ([47.19.105.250]) by smtp.gmail.com with ESMTPSA id f66sm683127qkc.25.2017.10.24.12.35.42 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 24 Oct 2017 12:35:42 -0700 (PDT) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A5C11CD262 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=fail (p=none dis=none) header.from=datto.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com A5C11CD262 Authentication-Results: mx1.redhat.com; dkim=fail reason="signature verification failed" (2048-bit key) header.d=datto-com.20150623.gappssmtp.com header.i=@datto-com.20150623.gappssmtp.com header.b="EDwbGCCy" DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A46776A7DF Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=pass (p=none dis=none) header.from=datto.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=dzamirski@datto.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com A46776A7DF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=datto-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=kia/pzGsaZim+j2VFw3+GUA/P4XWILZAGRBYO4Fq8kY=; b=EDwbGCCycISiRj1PXnHh8b9KR5YL7xO9LlIZCsZHIjIG4g1V/UjMD5+A+rTRSUsDiY zFD1El9w47kIdD1o1U3WWLmoloJAq0sh/75wREkWjLTlk0D13pQ5GN0NfiTde4H0kj3d SkY4UPhMDBGeC3yuYPM0RFPmW10rfA8t9qktTFDcebjh9MR9GcPLWqZPg3Kv9qVQ8RJJ qOfbaqTLsCuCvQbPGSjKdyc6OqcK4lhO4B1DugfiuhM2BHmXwSgaCKE0+tfjsGL2gs+Z 5no0vmPsSQf6GamtcDwsyJKdQ5oJ/4At2cjx2WYD7AVe6ynx5GGS2YgZprn+APfINWk0 JWUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=kia/pzGsaZim+j2VFw3+GUA/P4XWILZAGRBYO4Fq8kY=; b=gfCHCLn3grmUlUX1eiTRz3edcRLt/ciFotsCQ9z6u/vwHFD+y6kCZb+HRGVodFmmzz pHxoOXDPzEE0buRpqOfI8SBMu5ql3809whj3j32NATWeb+ZmPoHrmR2pEpturZfSkAW2 zb923bmE0oyCVZaV0YQm2ZPJ2TP+mOLPtzcbXda0P8ziIBskqLWaZVOHIB8NTMaGRMWU 591QMZHvwqO4vmp6d+QuHnJZoKPBsU3Cg8cI5I8lLf6qlE8S73HUUMK2OQzeMkaAvthD iuJ7Q5YBr2giZdjSnoYImcE4wxRiNFIEzePRZ8IWl18ZPfIbztMldKEddyRrX7dlGfvR cWdg== X-Gm-Message-State: AMCzsaWc5fdYmbnjUFXoa38NonT6ga2oMnXU4XGt8y2thurFkgc1MQOu e5mqge7IfETp8+6FOGB6fmuWs3XpD3Y= X-Google-Smtp-Source: ABhQp+T5zfU6hw7HM21b4DiyTyEp/fiScNBvC6aY6AGLWUEpibTmpuGmLrrev9tKBsfFOo+uX5ZfYg== X-Received: by 10.200.46.58 with SMTP id r55mr27163877qta.244.1508873742582; Tue, 24 Oct 2017 12:35:42 -0700 (PDT) From: Dawid Zamirski To: libvir-list@redhat.com Date: Tue, 24 Oct 2017 15:35:30 -0400 Message-Id: <20171024193538.9078-8-dzamirski@datto.com> In-Reply-To: <20171024193538.9078-1-dzamirski@datto.com> References: <20171024193538.9078-1-dzamirski@datto.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 24 Oct 2017 19:35:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 24 Oct 2017 19:35:44 +0000 (UTC) for IP:'209.85.216.179' DOMAIN:'mail-qt0-f179.google.com' HELO:'mail-qt0-f179.google.com' FROM:'dzamirski@datto.com' RCPT:'' X-RedHat-Spam-Score: -0.221 (DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_SORBS_SPAM, SPF_PASS) 209.85.216.179 mail-qt0-f179.google.com 209.85.216.179 mail-qt0-f179.google.com X-Scanned-By: MIMEDefang 2.78 on 10.5.110.27 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 07/15] vbox: Support empty removable drives. X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 24 Oct 2017 19:36:17 +0000 (UTC) X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Original code was checking for non empty disk source before proceeding to actually attach disk device to VM. This prevented from creating empty removable devices like DVD or floppy. Therefore, this patch re-organizes the loop work-flow to allow such configurations as well as makes the code follow better libvirt practices. Additionally, adjusted debug logs to be more helpful - removed old ones and added new which give more valuable info for troubleshooting. Reviewed-by: John Ferlan --- src/vbox/vbox_common.c | 206 +++++++++++++++++++++++++++++++--------------= ---- 1 file changed, 130 insertions(+), 76 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 9f4bf18a3..2bd891efb 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -959,11 +959,12 @@ static int vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr data, IMachine *machin= e) { size_t i; - int type, format, ret =3D 0; + int type, ret =3D 0; const char *src =3D NULL; nsresult rc =3D 0; virDomainDiskDefPtr disk =3D NULL; PRUnichar *storageCtlName =3D NULL; + char *controllerName =3D NULL; IMedium *medium =3D NULL; PRUnichar *mediumFileUtf16 =3D NULL; PRUint32 devicePort, deviceSlot, deviceType, accessMode; @@ -1015,47 +1016,104 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPt= r data, IMachine *machine) disk =3D def->disks[i]; src =3D virDomainDiskGetSource(disk); type =3D virDomainDiskGetType(disk); - format =3D virDomainDiskGetFormat(disk); deviceType =3D DeviceType_Null; accessMode =3D AccessMode_ReadOnly; devicePort =3D disk->info.addr.drive.unit; deviceSlot =3D disk->info.addr.drive.bus; =20 - VIR_DEBUG("disk(%zu) type: %d", i, type); - VIR_DEBUG("disk(%zu) device: %d", i, disk->device); - VIR_DEBUG("disk(%zu) bus: %d", i, disk->bus); - VIR_DEBUG("disk(%zu) src: %s", i, src); - VIR_DEBUG("disk(%zu) dst: %s", i, disk->dst); - VIR_DEBUG("disk(%zu) driverName: %s", i, - virDomainDiskGetDriver(disk)); - VIR_DEBUG("disk(%zu) driverType: %s", i, - virStorageFileFormatTypeToString(format)); - VIR_DEBUG("disk(%zu) cachemode: %d", i, disk->cachemode); - VIR_DEBUG("disk(%zu) readonly: %s", i, (disk->src->readonly - ? "True" : "False")); - VIR_DEBUG("disk(%zu) shared: %s", i, (disk->src->shared - ? "True" : "False")); - - if (type =3D=3D VIR_STORAGE_TYPE_FILE && src) { - VBOX_UTF8_TO_UTF16(src, &mediumFileUtf16); + if (type !=3D VIR_STORAGE_TYPE_FILE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported storage type %s, the only suppor= ted " + "type is %s"), + virStorageTypeToString(type), + virStorageTypeToString(VIR_STORAGE_TYPE_FILE)); + ret =3D -1; + goto cleanup; + } =20 - if (disk->device =3D=3D VIR_DOMAIN_DISK_DEVICE_DISK) { - deviceType =3D DeviceType_HardDisk; - accessMode =3D AccessMode_ReadWrite; - } else if (disk->device =3D=3D VIR_DOMAIN_DISK_DEVICE_CDROM) { - deviceType =3D DeviceType_DVD; - accessMode =3D AccessMode_ReadOnly; - } else if (disk->device =3D=3D VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - deviceType =3D DeviceType_Floppy; - accessMode =3D AccessMode_ReadWrite; - } else { + switch ((virDomainDiskDevice) disk->device) { + case VIR_DOMAIN_DISK_DEVICE_DISK: + if (!src) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Missing disk source file path")); ret =3D -1; goto cleanup; } =20 - gVBoxAPI.UIVirtualBox.FindHardDisk(data->vboxObj, mediumFileUt= f16, - deviceType, accessMode, &me= dium); + deviceType =3D DeviceType_HardDisk; + accessMode =3D AccessMode_ReadWrite; + + break; + + case VIR_DOMAIN_DISK_DEVICE_CDROM: + deviceType =3D DeviceType_DVD; + accessMode =3D AccessMode_ReadOnly; + + break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + deviceType =3D DeviceType_Floppy; + accessMode =3D AccessMode_ReadWrite; + + break; + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The vbox driver does not support %s disk dev= ice"), + virDomainDiskDeviceTypeToString(disk->device)); + ret =3D -1; + goto cleanup; + } + + switch ((virDomainDiskBus) disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + VBOX_UTF8_TO_UTF16("IDE Controller", &storageCtlName); + devicePort =3D def->disks[i]->info.addr.drive.bus; + deviceSlot =3D def->disks[i]->info.addr.drive.unit; + + break; + case VIR_DOMAIN_DISK_BUS_SATA: + VBOX_UTF8_TO_UTF16("SATA Controller", &storageCtlName); + + break; + case VIR_DOMAIN_DISK_BUS_SCSI: + VBOX_UTF8_TO_UTF16("SCSI Controller", &storageCtlName); + + break; + case VIR_DOMAIN_DISK_BUS_FDC: + VBOX_UTF8_TO_UTF16("Floppy Controller", &storageCtlName); + devicePort =3D 0; + deviceSlot =3D disk->info.addr.drive.unit; + + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The vbox driver does not support %s bus type= "), + virDomainDiskBusTypeToString(disk->bus)); + ret =3D -1; + goto cleanup; + } =20 + /* If disk source is specified, lookup IMedium - removable drives = don't + * have either. + */ + if (src) { + VBOX_UTF8_TO_UTF16(src, &mediumFileUtf16); + VIR_DEBUG("Looking up medium %s, type: %d, mode: %d", src, + deviceType, accessMode); + + rc =3D gVBoxAPI.UIVirtualBox.FindHardDisk(data->vboxObj, mediu= mFileUtf16, + deviceType, accessMode= , &medium); + + /* The following is not needed for vbox 4.2+ but older version= s have + * distinct find and open operations where the former looks in= vbox + * media registry while the latter at storage location. In 4.2= +, the + * OpenMedium call takes care of both cases internally + */ if (!medium) { rc =3D gVBoxAPI.UIVirtualBox.OpenMedium(data->vboxObj, mediumFileUtf16, @@ -1065,7 +1123,7 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr d= ata, IMachine *machine) =20 if (!medium) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to attach the following disk/dvd/= floppy " + _("Failed to open the following disk/dvd/fl= oppy " "to the machine: %s, rc=3D%08x"), src, rc= ); ret =3D -1; goto cleanup; @@ -1080,57 +1138,53 @@ vboxAttachDrives(virDomainDefPtr def, vboxDriverPtr= data, IMachine *machine) ret =3D -1; goto cleanup; } + } =20 - if (disk->device =3D=3D VIR_DOMAIN_DISK_DEVICE_DISK) { - if (disk->src->readonly) { - gVBoxAPI.UIMedium.SetType(medium, MediumType_Immutable= ); - VIR_DEBUG("Setting harddisk to immutable"); - } else if (!disk->src->readonly) { - gVBoxAPI.UIMedium.SetType(medium, MediumType_Normal); - VIR_DEBUG("Setting harddisk type to normal"); - } + if (disk->device =3D=3D VIR_DOMAIN_DISK_DEVICE_DISK) { + if (disk->src->readonly) { + gVBoxAPI.UIMedium.SetType(medium, MediumType_Immutable); + VIR_DEBUG("Setting hard disk to immutable"); + } else if (!disk->src->readonly) { + gVBoxAPI.UIMedium.SetType(medium, MediumType_Normal); + VIR_DEBUG("Setting hard disk type to normal"); } =20 - if (disk->bus =3D=3D VIR_DOMAIN_DISK_BUS_IDE) { - VBOX_UTF8_TO_UTF16("IDE Controller", &storageCtlName); - devicePort =3D def->disks[i]->info.addr.drive.bus; - deviceSlot =3D def->disks[i]->info.addr.drive.unit; - } else if (disk->bus =3D=3D VIR_DOMAIN_DISK_BUS_SATA) { - VBOX_UTF8_TO_UTF16("SATA Controller", &storageCtlName); - } else if (disk->bus =3D=3D VIR_DOMAIN_DISK_BUS_SCSI) { - VBOX_UTF8_TO_UTF16("SCSI Controller", &storageCtlName); - } else if (disk->bus =3D=3D VIR_DOMAIN_DISK_BUS_FDC) { - VBOX_UTF8_TO_UTF16("Floppy Controller", &storageCtlName); - devicePort =3D 0; - deviceSlot =3D disk->info.addr.drive.unit; - } + } =20 - /* attach the harddisk/dvd/Floppy to the storage controller */ - rc =3D gVBoxAPI.UIMachine.AttachDevice(machine, - storageCtlName, - devicePort, - deviceSlot, - deviceType, - medium); + VBOX_UTF16_TO_UTF8(storageCtlName, &controllerName); + VIR_DEBUG("Attaching disk(%zu), controller: %s, port: %d, slot: %d= , " + "type: %d, medium: %s", i, controllerName, devicePort, + deviceSlot, deviceType, medium =3D=3D NULL ? "empty" := src); + VBOX_UTF8_FREE(controllerName); =20 - if (NS_FAILED(rc)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not attach the file as " - "harddisk/dvd/floppy: %s, rc=3D%08x"), sr= c, rc); - ret =3D -1; - goto cleanup; - } else { - DEBUGIID("Attached HDD/DVD/Floppy with UUID", &mediumUUID); - } - cleanup: - VBOX_MEDIUM_RELEASE(medium); - vboxIIDUnalloc(&mediumUUID); - VBOX_UTF16_FREE(mediumFileUtf16); - VBOX_UTF16_FREE(storageCtlName); + /* Attach the harddisk/dvd/Floppy to the storage controller, + * medium =3D=3D NULL is ok here + */ + rc =3D gVBoxAPI.UIMachine.AttachDevice(machine, + storageCtlName, + devicePort, + deviceSlot, + deviceType, + medium); =20 - if (ret < 0) - break; + if (NS_FAILED(rc)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not attach the file as " + "harddisk/dvd/floppy: %s, rc=3D%08x"), src, r= c); + ret =3D -1; + goto cleanup; + } else { + DEBUGIID("Attached HDD/DVD/Floppy with UUID", &mediumUUID); } + + cleanup: + VBOX_MEDIUM_RELEASE(medium); + vboxIIDUnalloc(&mediumUUID); + VBOX_UTF16_FREE(mediumFileUtf16); + VBOX_UTF16_FREE(storageCtlName); + + if (ret < 0) + break; } =20 return ret; --=20 2.14.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list