Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ovirt-img: add upload-disk #108

Merged
merged 4 commits into from
Aug 1, 2022

Conversation

aesteve-rh
Copy link
Member

@aesteve-rh aesteve-rh commented Jul 27, 2022

Add upload-disk sub-command for upload disks.

$ ./ovirt-img upload-disk -h
usage: ovirt-img upload-disk [-h] [-c CONFIG] [--engine-url ENGINE_URL]
                             [--username USERNAME]
                             [--password-file PASSWORD_FILE] [--cafile CAFILE]
                             [--log-file LOG_FILE] [--log-level LOG_LEVEL]
                             [--max-workers MAX_WORKERS]
                             [--buffer-size BUFFER_SIZE] -s STORAGE_DOMAIN
                             [-f {raw,qcow2}] [--preallocated] [--disk-id ID]
                             filename

positional arguments:
  filename              Path to image to upload. Supported formats: raw,
                        qcow2, iso.

optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG, --config CONFIG
                        If set, read specified section from the configuration
                        file.
  --engine-url ENGINE_URL
                        ovirt-engine URL. If not set, read from the specified
                        config section (required).
  --username USERNAME   ovirt-engine username. If not set, read from the
                        specified config section (required).
  --password-file PASSWORD_FILE
                        Read ovirt-engine password from file. If not set, read
                        password from the specified config section, or prompt
                        the user for the password.
  --cafile CAFILE       Path to ovirt-engine CA certificate. If not set, read
                        from the specified config section
  --log-file LOG_FILE   Log to file instead of stderr.
  --log-level LOG_LEVEL
                        Log level (choices: {debug, info, warning, error},
                        default: warning).
  --max-workers MAX_WORKERS
                        Maximum number of workers (range: 1-8, default: 4).
  --buffer-size BUFFER_SIZE
                        Buffer size per worker (range: 64k-16m, default: 4m).
  -s STORAGE_DOMAIN, --storage-domain STORAGE_DOMAIN
                        Name of the storage domain.
  -f {raw,qcow2}, --format {raw,qcow2}
                        Upload image format (default qcow2 for data disks and
                        raw for iso disks).
  --preallocated        Create preallocated disk. Required when using raw
                        format on block based storage domain (iSCSI, FC). ISO
                        images are always uploaded to preallocated disk.
--disk-id ID       A UUID for the new disk. If not specified oVirt will
                        create a new UUID.
$ ./ovirt-img upload-disk --config engine --storage-domain iscsi-sd disk1.qcow2
[ ----  ] 0 bytes, 0.00 s, 0 bytes/s | inspecting image
[ ----  ] 0 bytes, 0.01 s, 0 bytes/s | creating disk
[ ----  ] 0 bytes, 6.39 s, 0 bytes/s | creating transfer
[  38%  ] 2.29 GiB, 15.99 s, 146.59 MiB/s | uploading image
[ 100%  ] 6.00 GiB, 22.88 s, 268.56 MiB/s | finalizing transfer
[ 100%  ] 6.00 GiB, 33.02 s, 186.07 MiB/s | upload completed

Possible combinations:

Content Storage Backup Format Allocation Allowed
data block yes qcow2 sparse ✔️
data block yes qcow2 preallocated ✔️
data block no raw sparse
data block no raw preallocated ✔️
data file yes qcow2 sparse ✔️
data file yes qcow2 preallocated ✔️
data file no raw sparse ✔️
data file no raw sparse ✔️
iso block no raw preallocated ✔️
iso file no raw preallocated ✔️

Note: Source allocation policies omitted as they make no difference.

Related: #72
Signed-off-by: Albert Esteve aesteve@redhat.com

@aesteve-rh aesteve-rh added this to the ovirt-4.5.2 milestone Jul 27, 2022
@aesteve-rh aesteve-rh requested a review from rokkbert July 27, 2022 12:43
@aesteve-rh aesteve-rh requested a review from nirs as a code owner July 27, 2022 12:43
@aesteve-rh aesteve-rh self-assigned this Jul 27, 2022
@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch 2 times, most recently from 53ebfc5 to 933701f Compare July 27, 2022 13:00
@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch from 933701f to cd07ad9 Compare July 28, 2022 12:52
@aesteve-rh
Copy link
Member Author

Just pushed to see how it looks in the PR and be able to close some threads and have a cleaner view of what's done. I want to see if I can add some tests for the new option types.

@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch 2 times, most recently from ef8ecc5 to ad6b767 Compare July 28, 2022 15:22
@aesteve-rh aesteve-rh requested a review from nirs July 28, 2022 15:24
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added note about the incremental backup flag.

@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch 2 times, most recently from c5dbb24 to ed1e3a0 Compare July 29, 2022 09:04
@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch from ed1e3a0 to 8c2f1ee Compare July 29, 2022 15:26
@aesteve-rh aesteve-rh requested review from nirs and rokkbert July 29, 2022 15:57
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch 3 times, most recently from 3c1301e to 8df7ece Compare August 1, 2022 09:03
@aesteve-rh
Copy link
Member Author

@nirs I followed all comments from the last review, you can see the most relevant changeset here: https://github.com/oVirt/ovirt-imageio/compare/8c2f1eecf04dc02da026173d5a7299d90708a20b..4267e85f6b6a99a7cc0a7bb7d7f80c09bbd1f21c

The other pushes were only correcting commit messages.

Table updated with content type for depicting ISO images. Except raw sparse on block storage, everything else is allowed with default arguments, so I think we are in a pretty good state!

@aesteve-rh aesteve-rh requested a review from nirs August 1, 2022 09:08
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch 2 times, most recently from 7302df9 to 5587dd4 Compare August 1, 2022 11:18
@aesteve-rh
Copy link
Member Author

aesteve-rh commented Aug 1, 2022

I did one more split in the _probe_image function (now renamed to _prepare), on top of all the changes in the last review.

I hope the code is improved and not worsened with the change, but it makes _prepare function look a bit better.

Changes: https://github.com/oVirt/ovirt-imageio/compare/8df7ece24895efc0fdf432a86d4854deef16798e..7302df95e96cbd109c850a026dd93c0390dcf543

@aesteve-rh aesteve-rh requested a review from nirs August 1, 2022 11:34
Copy link
Member

@nirs nirs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue with iso images.

Add a new "add_disk" function to the _ovirt
module for the ovirt-img tool. Add disk allows
to create a new disk in the engine through an
open connection.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add UUID type for UUID normalization and parameter
validation. Use it to verify disk_id in download-disk.

Output:
ovirt-img download-disk: error: argument disk_id:
    invalid UUID value: 'invalid-uuid-argument'

Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add an additional argument to the options.add_sub_command
function to optionally (True by default) add common
transfer-related options to the command.

Currently these options are "--max-workers" and
"--buffer-size".

Signed-off-by: Albert Esteve <aesteve@redhat.com>
Add upload-disk sub-command for upload disks.

$ ./ovirt-img upload-disk -h
usage: ovirt-img upload-disk [-h] [-c CONFIG] [--engine-url ENGINE_URL]
                             [--username USERNAME]
                             [--password-file PASSWORD_FILE] [--cafile CAFILE]
                             [--log-file LOG_FILE] [--log-level LOG_LEVEL]
                             [--max-workers MAX_WORKERS]
                             [--buffer-size BUFFER_SIZE] -s STORAGE_DOMAIN
                             [-f {raw,qcow2}] [--preallocated] [--disk-id ID]
                             filename

positional arguments:
  filename              Path to image to upload. Supported formats: raw,
                        qcow2, iso.

optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG, --config CONFIG
                        If set, read specified section from the configuration
                        file.
  --engine-url ENGINE_URL
                        ovirt-engine URL. If not set, read from the specified
                        config section (required).
  --username USERNAME   ovirt-engine username. If not set, read from the
                        specified config section (required).
  --password-file PASSWORD_FILE
                        Read ovirt-engine password from file. If not set, read
                        password from the specified config section, or prompt
                        the user for the password.
  --cafile CAFILE       Path to ovirt-engine CA certificate. If not set, read
                        from the specified config section
  --log-file LOG_FILE   Log to file instead of stderr.
  --log-level LOG_LEVEL
                        Log level (choices: {debug, info, warning, error},
                        default: warning).
  --max-workers MAX_WORKERS
                        Maximum number of workers (range: 1-8, default: 4).
  --buffer-size BUFFER_SIZE
                        Buffer size per worker (range: 64k-16m, default: 4m).
  -s STORAGE_DOMAIN, --storage-domain STORAGE_DOMAIN
                        Name of the storage domain.
  -f {raw,qcow2}, --format {raw,qcow2}
                        Upload image format (default qcow2 for data disks and
                        raw for iso disks).
  --preallocated        Create preallocated disk. Required when using raw
                        format on block based storage domain (iSCSI, FC). ISO
                        images are always uploaded to preallocated disk.
  --disk-id ID          A UUID for the new disk. If not specified oVirt will
                        create a new UUID.

$ ./ovirt-img upload-disk --config engine --storage-domain iscsi-sd disk1.qcow2
[ ----  ] 0 bytes, 0.00 s, 0 bytes/s | inspecting image
[ ----  ] 0 bytes, 0.01 s, 0 bytes/s | creating disk
[ ----  ] 0 bytes, 6.39 s, 0 bytes/s | creating transfer
[  38%  ] 2.29 GiB, 15.99 s, 146.59 MiB/s | uploading image
[ 100%  ] 6.00 GiB, 22.88 s, 268.56 MiB/s | finalizing transfer
[ 100%  ] 6.00 GiB, 33.02 s, 186.07 MiB/s | upload completed

Possible combinations:
Content | Storage | Backup | Format | Allocation   | Allowed
------- | ------- | ------ | ------ | ------------ | -------
data    | block   | yes    | qcow   | sparse       | ✓
data    | block   | yes    | qcow   | preallocated | ✓
data    | block   | no     | raw    | sparse       | x
data    | block   | no     | raw    | preallocated | ✓
data    | file    | yes    | qcow   | sparse       | ✓
data    | file    | yes    | qcow   | preallocated | ✓
data    | file    | no     | raw    | sparse       | ✓
data    | file    | no     | raw    | sparse       | ✓
iso     | block   | no     | raw    | preallocated | ✓
iso     | file    | no     | raw    | preallocated | ✓

Note: Source allocation policies omitted as they make no difference.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
@aesteve-rh aesteve-rh force-pushed the aesteve/add-upload-disk-cmd branch from 5587dd4 to 7e39962 Compare August 1, 2022 12:15
@nirs nirs merged commit a7b9266 into oVirt:master Aug 1, 2022
@aesteve-rh aesteve-rh deleted the aesteve/add-upload-disk-cmd branch August 23, 2022 07:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants