From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from GVXPR05CU001.outbound.protection.outlook.com (mail-swedencentralazon11023130.outbound.protection.outlook.com [52.101.83.130]) by lore.virtuozzo.com (Postfix) with ESMTPS id 926B580266 for ; Thu, 3 Sep 2026 15:34:48 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=LXwahtEJ3CP3rIpw2hjeQSaG5opwOtpr1Pdp3IiQs617x6U5IR51lXhaiwHQBYbYtGaVaKnxa3pZMAIMk/ASu9gT7ecgKd3jkeyO0x8+3KbMYMKLGtuNP8IDmPgKVvP+Rh72QUYTkI7bhbTTJiNyR9qT8fT9M7Z6miZq3bPpHwbDEwMjBppGh3VTedMa6N/soH7eaKBCNiL8a7CuuhQqvUk2yTt/O7rbX5M9BiK9ULlzC2j9kMeP8QRmuCtVqfgMKLr7KGQHWLQ2Sx1zBGKUwZymnydP4K8ZAmvV8GkPRnWHYK0lPGM5OLOFOREMEXMszAO9bRS5P9Q8QhwRBIsstw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=94Ltxrl77PJxGRLVd468yd9u6jb10QMMLE3TEbbpLpg=; b=sI2/PdPll14PmUrTb5c2GNYWEFEUAAqDfMTmNngJIrN9LjotnkZX8w9zBZpn9xFZRQUic1hsEk+yo7hxX2L561LtbWCjJRkSb4tR65L9oPjWt5UwmO+Oo+Yzh/OZchWNhavow6fqaDwzbfyusFKgw6IQC9cyijvU3TPjyZyOd+McuUopTWVNjRcXA8hPQAW4M8QYzNTsUUznthfCSaxLwWn1nLD76x+nx2sTLqT0NN7G98Z0heOiyka96ytWtonnMnV9hKIi26FCTEibU3Yb5qasQap67RMWUw/Fq8POxLcVgFWASnSRcxAgm1woV+jUWQeEBghfso2bIEtcafnPpA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=virtuozzo.com; dmarc=pass action=none header.from=virtuozzo.com; dkim=pass header.d=virtuozzo.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=virtuozzo.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=94Ltxrl77PJxGRLVd468yd9u6jb10QMMLE3TEbbpLpg=; b=Hszcjy/DNUif2tQWxA9mDcTiXL7WsgCMBiiE46gec+Y+n8No1nC0SC6OhAs0TZN1BvA+zDOgOhItgCLBUs4ecKQg3q26mEyXjxrfMxGc6va6O1TScOmIx+Wkhacsr7BrFws+OI6A8BL2bQKUEa5DjeOlv9HkA4FI85AucgpkQkltq7HMMXZlu8sgVRt7pZonU22ELRYuzASBdvsSy5G+BUMjRjtDl0/k7W7MSeK/+Xu8aCt18VzWshxgdEa75aDl7gJTYQGbhQA2PqgysDUYVZLu2AdWzIlsI0CI7vnfxHj47PjoN+AKb9RgGX/EqzefytsOLggC1LH+hzElzs6Z0w== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=virtuozzo.com; Received: from VI0PR08MB10656.eurprd08.prod.outlook.com (2603:10a6:800:20a::12) by DBAPR08MB5733.eurprd08.prod.outlook.com (2603:10a6:10:1b3::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.21.382.9; Thu, 3 Sep 2026 15:34:46 +0000 Received: from VI0PR08MB10656.eurprd08.prod.outlook.com ([fe80::4e37:b189:ddcd:3dd8]) by VI0PR08MB10656.eurprd08.prod.outlook.com ([fe80::4e37:b189:ddcd:3dd8%7]) with mapi id 15.21.0382.007; Thu, 3 Sep 2026 15:34:46 +0000 Message-ID: <881fb851-e640-4e80-bdcf-c0dfd972a3f8@virtuozzo.com> Date: Thu, 3 Sep 2026 18:34:44 +0300 User-Agent: Mozilla Thunderbird Subject: Re: [QEMU HCI-8.0 PATCH 2/5] vhost-blk: change backend setup To: Andrey Zhadchenko Cc: svt-core@virtuozzo.com, den@openvz.org References: <20260903123204.24035-1-andrey.zhadchenko@virtuozzo.com> <20260903123204.24035-3-andrey.zhadchenko@virtuozzo.com> <178844736480.581266.3084182582245534198.b4-review@b4> <26935de1-a678-47d7-b7f7-81543b041c68@virtuozzo.com> Content-Language: en-US From: Andrey Drobyshev In-Reply-To: <26935de1-a678-47d7-b7f7-81543b041c68@virtuozzo.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: ZR2P278CA0055.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:53::9) To VI0PR08MB10656.eurprd08.prod.outlook.com (2603:10a6:800:20a::12) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VI0PR08MB10656:EE_|DBAPR08MB5733:EE_ X-MS-Office365-Filtering-Correlation-Id: 6f861204-1afa-4554-bde8-08df09d0e1c2 X-LD-Processed: 0bc7f26d-0264-416e-a6fc-8352af79c58f,ExtAddr List-Id: svt-core@virtuozzo.com X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|1800799024|366016|376014|23010399003|5113699003|6133799003|10067099003|56012099006|4143699003|18002099003|22082099003; X-Microsoft-Antispam-Message-Info: IYrV+OlymlvJFn25b1x60KoYKSTv9f+dPtcp1rv+e+Nr5KgdPXeh8ET5Ac4aWscU5pMULIPKjVvZIjUvCCcoahuQq5oOAN7UtH9lPqmfBIgtlrEzNlnnC/fKSFCjjm4Nn6aQg+77Wx2FgaAvWuFNvlmPWFw+aoNQA/4zLCtYtvmgJdCIJDM0sWZOhfuCD7V5+UXztfq9JJdkZbBGvztCekEDd4uaV0vROvcyOX2Ud/dMxbvSa6H2+FfKIHgUss4rLoKNhGPIpj2IKiSDKKlR4GiUTsVdvStg0muw51PMiP2BWImBDquGAqZG7Fkg335wQ2EKphcdaP/0Qm1sHntoftWQUJMEdxeo+FUzFQG8Wq6w3txEho1GhjCevFgcRZwjG/2UMTF9hgVHjmLh9+qx4YMoE2IiNhD47WsD+gY8mKTP6iu//v9t8zTJzYsvnw0Yx82ktS8QcaCeWEKqY13LTjWc3prz6ILG19NigBHsNSKlpnqfjd4jJXs8sQFVN7bttVHs+LkV21HqHpLfwzwhmwORdHS9wyPFuwyAp/Nrlp0yg3XcqDIv+/lo5YvxzUdpdon394I/p2TNM2C/e/1Ck3yhVz/CRngsriaARJA4CG8Uz+BtzJU1vtx1nTx2QilPpczeEVZuuGCWUbjknLdfNxBF2YAETh5X9bVQYYB+67Q= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI0PR08MB10656.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(1800799024)(366016)(376014)(23010399003)(5113699003)(6133799003)(10067099003)(56012099006)(4143699003)(18002099003)(22082099003);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RzNPUTJ4bjJpRDNjNFR0Sm1wOTd3TVhRelJuQ1NZdWFpVEpvTTZLSG9hMC8z?= =?utf-8?B?ZExtU1liekkzM3JHS2JNTVhlYUhJRGpxNm8xSHh3K01UWVZadzZ2TlRHNGxk?= =?utf-8?B?RXVXWUx1WFB0dUlpbXlHMXVhVmN3aDlqZEgwV3JGb3huWmw4aG9VQWZ6bVRq?= =?utf-8?B?SzVMeUdsVkN5WkY1VytvdVA0Q2ZPQTBnUHlaMzBVa3luaFkvSGtkN3k3QnJQ?= =?utf-8?B?NktGVDFHZ0F6Wjg5ZEoxU21zSTRyL0d1dFRQYjJjbWV3TERDa05TYVdWYU1U?= =?utf-8?B?WTJZNkljenowWnJRaTg5MjBkWDVYZlpQL0dTTDJDQ09ySmZsdUkvei92ZFc2?= =?utf-8?B?eDIxUHhPSTEzTlZwTUtYMEVieWhhSU03dEtiek9MMmQwUk1nVnFXL3ZRaTBi?= =?utf-8?B?bUFIczF1QjNkV0tGSWdPOUNPQVZSVWk2N1pQbk5ickcrelNaaVJSOUtxNEdU?= =?utf-8?B?V1lCdzYzeVRmbWxEalJiSmFyK3JXWUcyeVJjUDhNUDcwTnh1cmJSVTJzT01k?= =?utf-8?B?YloxOXpOemw5anhVZGQzVkFrRHBXVzVkRkJpLzBvKzFwYTUyU1QvNEp4Z010?= =?utf-8?B?azJMRVdndmUxaDVwVkpsd2pEQXpqS3RzSVRheU1ubjZXTDNhbkZXUzVhRDlT?= =?utf-8?B?OFhaL21ZN3ZOR0x1U1R3K0dlTUhsMk4xa0ZpV0FteXd5ZllMSjNIdWk4NXEr?= =?utf-8?B?cGFhOVNnd3UzZ3Jiek1wTC9LTWdISUJ4U2FDZWpMcHBmMnFpVXoxM3lHVk9N?= =?utf-8?B?QUthdDFBaVBZbGFoSEZValZpMWhxbWhabGlxaG5CaW4rL1djeUpwWmlRbjhV?= =?utf-8?B?eUhLZmVxMklmdXMyZHZnTjVGQ1gyc2E2TFU5WHVmZjJNSmFBRGVSeHh0Ykty?= =?utf-8?B?VGVVMUkvV2hWTnpkYkRlS2xiTkplK0FsZUx2eWMyUnoxSVJ6TWVBWW1FelZZ?= =?utf-8?B?K2lhRVN0L0VTNktKcVo1ZzFIVkJ1VHVVdGxvQm5xYmNab0ZIQXQrT2swVkpT?= =?utf-8?B?R3RYTjF3aysvVGZQaXRZZlgyRVJ6c0ROcEhHaE54ZzRGZ3oxTXlpelAxTUt1?= =?utf-8?B?VWMwbmxkbXZKeW5XekM2NmFLZXZ5NWIrajlhOEt6VXVWTVRsRWUyRzFiTWhN?= =?utf-8?B?akFqMDJibjZjVnlxcnhCbHgyRGZmV1JsWVBFcFN6UHE2UGF5YmU5UTU1YzJC?= =?utf-8?B?ajhBMTJkK0VLMnhlaEFnYnp4YnpHL3Q1TExZTkJyTkFvVml4dnJsRWM3dnR3?= =?utf-8?B?UmVFVGtnMDNMWDU0ckhMTklOdEI1c3FsM0ZBTTV6VzN3NkkzOVQ4N1RLeWhk?= =?utf-8?B?ZlBKYlVKUjI3KzFwanh3cDlTc1djcWNuSDZTSmM5T2RrRCtzM01wWHlxTDRC?= =?utf-8?B?RlZEZVIveWE5ZUtidVVDYnp2M3piWmF2TlZwdlhtYkIzUlRmSjJORExndXhJ?= =?utf-8?B?aFQ2SHZVOUNaQzgraTNrQjAzWG5GZDFIdWZNbHkyYTFJQUUxVnNrUHU4UERy?= =?utf-8?B?RjQwb0k0cVBIUlJSUG92RWRmM2FyQmFkN2lrdmFPeExza0NoZWhidGRuRkNN?= =?utf-8?B?VDV2eU5sMnM1ZlVKR0pzZ2hGa1VjeVJTTUZyVVBESWxOOVl1VTJaUHk5Q3pF?= =?utf-8?B?SGErT2hGNkM0UytsdHozZTNNRFVNU00zSmlzME4yRElrZFMySlFaR3M3Y1dk?= =?utf-8?B?VUN2eWkzUjZzMzNUUzZMcm95R2FaWkpLcUthL0dITE1FV3FiTnBXTWJvQWJ1?= =?utf-8?B?MEVJYlZJLzlMSzJvRlhJMXNCUzBWQWVwMXFDK0lzalZhQitDMDZnSC91dzVK?= =?utf-8?B?UERwcklEVlU3dVJzc25lYk56ZDYyV2VHYzFBOEliWnJmN1NRS3AzbW00ZHhU?= =?utf-8?B?cFhobUZmTUpiMTdCcSttMDhpRkFBeTJUSXRqQUlETVlRaW45OXJ0Vm1jQW80?= =?utf-8?B?WnBpbmJJbk4vVkI5VXdFQ0w4ekpVa1VjRDZlQWhCRHdJNklxZ2daVHgxRmU0?= =?utf-8?B?bmlEZkRWN0NDSHgvdmVHaGE4VU1IdE1BeEo3ZXZtREU0YjFzbzdMWjY5anU2?= =?utf-8?B?YmxSWXpCUU9SR1ZhTTBJWm5NeTV6WjZPcFVOcFRJYlRFWGYvMlYxeVhic3h1?= =?utf-8?B?MDEzTTdWdEpVN0t1aGpmaXlzK1lvS1ozTkpheVdrZVN0RHhJTWdUT01oZWNl?= =?utf-8?B?aGhOK0I4ZFRkOHp5aHJ4VkVwQzFoc1A4eHVWS0FwenBQVHZsdFZoRUV5YWF2?= =?utf-8?B?UWVVMkNKcVZtVzYwZWtKRHVrNUt0dVN5cTNBRXMxcE11MnlVaXNOZ3d2bFMx?= =?utf-8?B?NTFVakNwdGZpa2hOOTBydEM5VWtxd3FDTmxRNWFHMU1GSDVwWVFyL1Q3U3pO?= =?utf-8?Q?946DXcXUS3aODkpA=3D?= X-Auto-Response-Suppress: DR, OOF, AutoReply X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6f861204-1afa-4554-bde8-08df09d0e1c2 X-MS-Exchange-CrossTenant-AuthSource: VI0PR08MB10656.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Sep 2026 15:34:45.8247 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 0bc7f26d-0264-416e-a6fc-8352af79c58f X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: zNww1xkZ0gAB8fxiYFP1ekk9+C13Za5KMRHlQ4K55xXY9Lu0kHjTi8fTBYvOYdmARLqsUlcKIEjKmmCYWAHy899GffV00X6hNydhjY3m7mQ= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBAPR08MB5733 On 9/3/26 6:27 PM, Andrey Zhadchenko wrote: > > > On 9/3/26 16:56, Andrey Drobyshev wrote: >>> Previously we used very ugly and incapsulation-breaking assignment >>> fd = blk_bs(s->conf.conf.blk)->file->bs->opaque; >>> It is wrong in a many ways, so let's rework this. >>> >>> Patch changes default `drive` to new `devpath` option so device fd >>> is managed by vhost-blk itself. Unfortunately this way we need a >>> bit more preparational work: finding out disk length, block size, >>> etc. Don't be too broad and just do the minimal and set reasonable >>> default values. Validate with previously introduced >>> blkconf_validate_blocksizes(). >>> Also we lose resize, as this is tied to the block node, which is >>> now have no place in the setup. We will add this in the next >>> patches as well as RO mode. >>> >>> https://virtuozzo.atlassian.net/browse/VSTOR-143437 >>> Signed-off-by: Andrey Zhadchenko >>> >>> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c >>> index c52851fcf8b..a3e0010982f 100644 >>> --- a/hw/block/vhost-blk.c >>> +++ b/hw/block/vhost-blk.c >>> @@ -24,23 +24,17 @@ >>> #include "system/system.h" >>> #include "linux-headers/linux/vhost.h" >>> #include >>> -#include >>> -#include "include/block/block_int-common.h" >>> #include "system/runstate.h" >>> >>> static int vhost_blk_start(VirtIODevice *vdev) >>> { >>> VHostBlk *s = VHOST_BLK(vdev); >>> struct vhost_vring_file backend; >>> - int ret, i, nworkers, *fd; >>> + int ret, i, nworkers; >>> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); >>> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >>> char serial[VIRTIO_BLK_ID_BYTES] = {0}; >>> >>> - bdrv_graph_rdlock_main_loop(); >>> - fd = blk_bs(s->conf.conf.blk)->file->bs->opaque; >>> - bdrv_graph_rdunlock_main_loop(); >>> - >>> if (!k->set_guest_notifiers) { >>> error_report("vhost-blk: binding does not support guest notifiers"); >>> return -ENOSYS; >>> @@ -92,7 +86,7 @@ static int vhost_blk_start(VirtIODevice *vdev) >>> >>> memset(&backend, 0, sizeof(backend)); >>> backend.index = 0; >>> - backend.fd = *fd; >>> + backend.fd = s->backend_fd; >>> if (ioctl(s->vhostfd, VHOST_BLK_SET_BACKEND, &backend)) { >>> error_report("vhost-blk: unable to set backend"); >>> ret = -errno; >>> @@ -208,29 +202,79 @@ static void vhost_blk_vm_state(void *opaque, bool running, RunState state) >>> } >>> } >>> >>> -static void vhost_blk_resize_cb(void *opaque) >>> +static int vhost_blk_update_size(VHostBlk *s, Error **errp) >>> { >>> - VirtIODevice *vdev = opaque; >>> + BlockConf *conf = &s->conf.conf; >>> + off_t length; >>> + bool changed; >>> + >>> + length = lseek(s->backend_fd, 0, SEEK_END); >>> + if (length < 0) { >>> + int error = errno; >>> + >>> + error_setg_errno(errp, error, >>> + "vhost-blk: unable to determine size of '%s'", >>> + s->conf.devpath); >>> + return -error; >>> + } >>> >>> - assert(qemu_get_current_aio_context() == qemu_get_aio_context()); >>> - virtio_notify_config(vdev); >>> + changed = s->length != length; >>> + s->length = length; >>> + conf->heads = 16; >>> + conf->secs = 63; >>> + conf->cyls = s->length / BDRV_SECTOR_SIZE / >>> + (conf->heads * conf->secs); >>> + conf->cyls = MIN(MAX(conf->cyls, 2U), 16383U); >>> + >>> + return changed; >> >> This function should return int, but here we return bool. And then >> we do 'if (vhost_blk_update_size() < 0) ...', which never fires. >> >>> } >>> >>> -static void vhost_blk_resize(void *opaque) >>> +static bool vhost_blk_open_backend(VHostBlk *s, Error **errp) >>> { >>> - VirtIODevice *vdev = VIRTIO_DEVICE(opaque); >>> + BlockConf *conf = &s->conf.conf; >>> + struct stat st; >>> >>> - /* >>> - * virtio_notify_config() needs to acquire the global mutex, >>> - * so it can't be called from an iothread. Instead, schedule >>> - * it to be run in the main context BH. >>> - */ >>> - aio_bh_schedule_oneshot(qemu_get_aio_context(), vhost_blk_resize_cb, vdev); >>> -} >>> + s->backend_fd = qemu_open(s->conf.devpath, O_RDWR, errp); >>> + if (s->backend_fd < 0) { >>> + error_prepend(errp, "vhost-blk: unable to open backend: "); >>> + return false; >>> + } >> >> Suggestion: how about also checking BLKSSZGET value of the device at this >> point and comparing it against conf->logical_block_size? > > I would personally avoid this for now. First of all BLKSSZGET (and other > things) may be undefined (failed this for BLKROGET btw) and proper ifdef > decoration are not so important for 'change backend setup' patch. We can > always add it later. > At least in our headers BLKROGET and BLKSSZGET are defined together. Both conf.readonly and conf.logical_block_size are set by the user (i.e. libvirt) and might mismatch with the actual device state. So IMHO they're symmetrical in this regard. >> >>> >>> -static const BlockDevOps vhost_blk_block_ops = { >>> - .resize_cb = vhost_blk_resize, >>> -}; >>> + if (fstat(s->backend_fd, &st) < 0) { >>> + error_setg_errno(errp, errno, "vhost-blk: unable to stat '%s'", >>> + s->conf.devpath); >>> + goto fail; >>> + } >>> + >>> + if (!S_ISBLK(st.st_mode)) { >>> + error_setg(errp, "vhost-blk: '%s' is not a block device", >>> + s->conf.devpath); >>> + goto fail; >>> + } >>> + >>> + if (vhost_blk_update_size(s, errp) < 0) { >>> + goto fail; >>> + } >>> + >>> + if (!conf->logical_block_size) { >>> + conf->logical_block_size = BDRV_SECTOR_SIZE; >>> + } >>> + >>> + if (!conf->physical_block_size) { >>> + conf->physical_block_size = BDRV_SECTOR_SIZE; >>> + } >>> + >>> + if (!blkconf_validate_blocksizes(conf, errp)) { >>> + goto fail; >>> + } >>> + >>> + return true; >>> + >>> +fail: >>> + qemu_close(s->backend_fd); >>> + s->backend_fd = -1; >>> + return false; >>> +} >>> >>> static void vhost_blk_device_realize(DeviceState *dev, Error **errp) >>> { >>> @@ -239,13 +283,8 @@ static void vhost_blk_device_realize(DeviceState *dev, Error **errp) >>> VhostBlkConf *conf = &s->conf; >>> int i, ret; >>> >>> - if (!conf->conf.blk) { >>> - error_setg(errp, "vhost-blk: drive property not set"); >>> - return; >>> - } >>> - >>> - if (!blk_is_inserted(conf->conf.blk)) { >>> - error_setg(errp, "vhost-blk: device needs media, but drive is empty"); >>> + if (!conf->devpath) { >>> + error_setg(errp, "vhost-blk: devpath property must be set"); >>> return; >>> } >>> >>> @@ -273,17 +312,7 @@ static void vhost_blk_device_realize(DeviceState *dev, Error **errp) >>> return; >>> } >>> >>> - if (!blkconf_apply_backend_options(&conf->conf, >>> - !blk_supports_write_perm(conf->conf.blk), >>> - true, errp)) { >>> - return; >>> - } >>> - >>> - if (!blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, errp)) { >>> - return; >>> - } >>> - >>> - if (!blkconf_blocksizes(&conf->conf, errp)) { >>> + if (!vhost_blk_open_backend(s, errp)) { >>> return; >>> } >>> >>> @@ -311,13 +340,13 @@ static void vhost_blk_device_realize(DeviceState *dev, Error **errp) >>> goto cleanup; >>> } >>> >>> - blk_set_dev_ops(s->conf.conf.blk, &vhost_blk_block_ops, s); >>> - >>> ret = vhost_dev_init(&s->dev, (void *)((size_t)s->vhostfd), >>> VHOST_BACKEND_TYPE_KERNEL, 0, NULL); >>> if (ret < 0) { >>> error_setg(errp, "vhost-blk: vhost initialization failed: %s", >>> strerror(-ret)); >>> + /* vhost_dev_init() closes vhostfd on failure */ >>> + s->vhostfd = -1; >> >> Before this patch we were doing double close(vhostfd) after vhost_dev_init() >> failure. I'd make it a separate commit with a "Fixes:" tag. >> >>> goto cleanup; >>> } >>> >>> @@ -328,7 +357,14 @@ cleanup: >>> qemu_del_vm_change_state_handler(s->mighand); >>> } >>> g_free(s->dev.vqs); >>> - close(s->vhostfd); >>> + if (s->vhostfd >= 0) { >>> + close(s->vhostfd); >>> + s->vhostfd = -1; >>> + } >>> + if (s->backend_fd >= 0) { >>> + qemu_close(s->backend_fd); >>> + s->backend_fd = -1; >>> + } >>> for (i = 0; i < conf->num_queues; i++) { >>> virtio_del_queue(vdev, i); >>> } >>> @@ -344,6 +380,10 @@ static void vhost_blk_device_unrealize(DeviceState *dev) >>> qemu_del_vm_change_state_handler(s->mighand); >>> vhost_blk_set_status(vdev, 0); >>> vhost_dev_cleanup(&s->dev); >>> + if (s->backend_fd >= 0) { >>> + qemu_close(s->backend_fd); >>> + s->backend_fd = -1; >>> + } >>> g_free(s->dev.vqs); >>> virtio_cleanup(vdev); >>> } >>> @@ -376,10 +416,6 @@ static uint64_t vhost_blk_get_features(VirtIODevice *vdev, >>> >>> virtio_add_feature(&features, VIRTIO_F_VERSION_1); >>> >>> - if (!blk_is_writable(s->conf.conf.blk)) { >>> - virtio_add_feature(&features, VIRTIO_BLK_F_RO); >>> - } >>> - >>> if (s->conf.num_queues > 1) { >>> virtio_add_feature(&features, VIRTIO_BLK_F_MQ); >>> } >>> @@ -398,7 +434,9 @@ static void vhost_blk_update_config(VirtIODevice *vdev, uint8_t *config) >>> int64_t length; >>> int blk_size = conf->logical_block_size; >>> >>> - blk_get_geometry(s->conf.conf.blk, &capacity); >>> + length = s->length; >>> + capacity = length / BDRV_SECTOR_SIZE; >>> + >>> memset(&blkcfg, 0, sizeof(blkcfg)); >>> virtio_stq_p(vdev, &blkcfg.capacity, capacity); >>> virtio_stl_p(vdev, &blkcfg.seg_max, s->conf.queue_size - 2); >>> @@ -406,7 +444,6 @@ static void vhost_blk_update_config(VirtIODevice *vdev, uint8_t *config) >>> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size); >>> blkcfg.geometry.heads = conf->heads; >>> >>> - length = blk_getlength(s->conf.conf.blk); >>> if (length > 0 && length / conf->heads / conf->secs % blk_size) { >>> unsigned short mask; >>> >>> @@ -425,7 +462,8 @@ static void vhost_blk_update_config(VirtIODevice *vdev, uint8_t *config) >>> } >>> >>> static const Property vhost_blk_properties[] = { >>> - DEFINE_BLOCK_PROPERTIES(VHostBlk, conf.conf), >>> + DEFINE_BLOCK_PROPERTIES_BASE(VHostBlk, conf.conf), >> >> DEFINE_BLOCK_PROPERTIES_BASE() macro defines lots of properties that >> make no sense without BlockBackend. E.g. backend_defaults, write-cache, >> share-rw, account-invalid, account-failed, stats-intervals. We should >> consider limiting the list of config properties to the ones which really >> matter to us. Ideally as a separate commit. > > From one point of view yes, from another point of view a lot other make > sense for virtio-device. I thought it was better to leave it as is and > use it later. > But maybe remove it altogether (also along with logical/physical block > size) and better add it later as separate options if we feel tuning > these values brings any impact? In general I'd just vote for limiting that list to the properties that we're actually using and that matter to us. Whether it's done via a new list or via limiting existing ones is technical details. Andrey