[libvirt] [PATCH v3 0/2] Couple of storage driver improvements
v3 of: https://www.redhat.com/archives/libvir-list/2019-March/msg00326.html diff to v2: - Patches acked in v2 are pushed now, - Patch 1/2 is a resurrection of patch 1/6 from v1. As Pavel found out we can't use writesame(), so let's simplify write() at least. - Patch 2/2 now checks for VSTORAGE too. Michal Prívozník (2): storage_backend_iscsi_direct: Simplify vol zeroing storageVolWipePattern: Don't take shortcut to refreshPool() src/storage/storage_backend_iscsi_direct.c | 26 ++++++++++++---------- src/storage/storage_driver.c | 15 ++++++++----- 2 files changed, 24 insertions(+), 17 deletions(-) -- 2.19.2
So far we have two branches: either we zero BLOCK_PER_PACKET (currently 128) block at once, or if we're close to the last block then we zero out one block at the time. This is very suboptimal. We know how many block are there left. Might as well just write them all at once. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_iscsi_direct.c | 26 ++++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 6283a2836b..5f2dfc0405 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -638,21 +638,23 @@ virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol, return ret; while (lba < nb_block) { - if (nb_block - lba > block_size * BLOCK_PER_PACKET) { + const uint64_t to_write = MIN(nb_block - lba + 1, BLOCK_PER_PACKET); - if (!(task = iscsi_write16_sync(iscsi, lun, lba, data, - block_size * BLOCK_PER_PACKET, - block_size, 0, 0, 0, 0, 0))) - return -1; - scsi_free_scsi_task(task); - lba += BLOCK_PER_PACKET; - } else { - if (!(task = iscsi_write16_sync(iscsi, lun, lba, data, block_size, - block_size, 0, 0, 0, 0, 0))) - return -1; + task = iscsi_write16_sync(iscsi, lun, lba, data, + to_write * block_size, + block_size, 0, 0, 0, 0, 0); + + if (!task || + task->status != SCSI_STATUS_GOOD) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to write to LUN %d: %s"), + lun, iscsi_get_error(iscsi)); scsi_free_scsi_task(task); - lba++; + return -1; } + + scsi_free_scsi_task(task); + lba += to_write; } return 0; -- 2.19.2
In d16f803d780 we've tried to solve an issue that after wiping an image its format might have changed (e.g. from qcow2 to raw) but libvirt wasn't probing the image format. We fixed this by calling virStorageBackendRefreshVolTargetUpdate() which is what refreshPool() would end up calling. But this shortcut is not good enough because the function is called only for local types of volumes (like dir, fs, netfs). But now that more backends support volume wiping we have to call the function with more caution. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_driver.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 95561a0578..496d51b1e0 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2572,11 +2572,16 @@ storageVolWipePattern(virStorageVolPtr vol, if (rc < 0) goto cleanup; - /* Instead of using the refreshVol, since much changes on the target - * volume, let's update using the same function as refreshPool would - * use when it discovers a volume. The only failure to capture is -1, - * we can ignore -2. */ - if (virStorageBackendRefreshVolTargetUpdate(voldef) == -1) + /* For local volumes, Instead of using the refreshVol, since + * much changes on the target volume, let's update using the + * same function as refreshPool would use when it discovers a + * volume. The only failure to capture is -1, we can ignore + * -2. */ + if ((backend->type == VIR_STORAGE_POOL_DIR || + backend->type == VIR_STORAGE_POOL_FS || + backend->type == VIR_STORAGE_POOL_NETFS || + backend->type == VIR_STORAGE_POOL_VSTORAGE) && + virStorageBackendRefreshVolTargetUpdate(voldef) == -1) goto cleanup; ret = 0; -- 2.19.2
On Sat, Mar 16, 2019 at 08:47:57AM +0100, Michal Privoznik wrote:
v3 of:
https://www.redhat.com/archives/libvir-list/2019-March/msg00326.html
diff to v2: - Patches acked in v2 are pushed now, - Patch 1/2 is a resurrection of patch 1/6 from v1. As Pavel found out we can't use writesame(), so let's simplify write() at least. - Patch 2/2 now checks for VSTORAGE too.
Michal Prívozník (2): storage_backend_iscsi_direct: Simplify vol zeroing storageVolWipePattern: Don't take shortcut to refreshPool()
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Michal Privoznik -
Pavel Hrdina