[libvirt] [PATCH] storage: Fix mention of disk pool default

Eric Blake posted 1 patch 5 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180829222217.286074-1-eblake@redhat.com
Test syntax-check passed
docs/storage.html.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH] storage: Fix mention of disk pool default
Posted by Eric Blake 5 years, 7 months ago
The default disk storage pool type is 'dos', not 'msdos'.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I ran into this doc bug when trying to figure out why a disk storage
pool that I had copied from another machine wouldn't autostart; it
turns out that the old disk used BIOS partitioning (dos), and the new
one uses UEFI (gpt).

I wonder if it would be nicer if a disk pool without a default
partition type supplied by the user could auto-detect the right
format, rather than defaulting to dos, but that's a bigger question
that I won't be tackling, and wouldn't have necessarily helped me
(if the <target> element was already present when I copied the file).

 docs/storage.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/storage.html.in b/docs/storage.html.in
index e9e6ec7423..6ec623831c 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -310,7 +310,7 @@
       on the size and placement of volumes. The 'free extents'
       information will detail the regions which are available for creating
       new volumes. A volume cannot span across 2 different free extents.
-      It will default to using <code>msdos</code> as the pool source format.
+      It will default to using <code>dos</code> as the pool source format.
     </p>

     <h3>Example pool input</h3>
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Fix mention of disk pool default
Posted by John Ferlan 5 years, 7 months ago

On 08/29/2018 06:22 PM, Eric Blake wrote:
> The default disk storage pool type is 'dos', not 'msdos'.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I ran into this doc bug when trying to figure out why a disk storage
> pool that I had copied from another machine wouldn't autostart; it
> turns out that the old disk used BIOS partitioning (dos), and the new
> one uses UEFI (gpt).
> 
> I wonder if it would be nicer if a disk pool without a default
> partition type supplied by the user could auto-detect the right
> format, rather than defaulting to dos, but that's a bigger question
> that I won't be tackling, and wouldn't have necessarily helped me
> (if the <target> element was already present when I copied the file).
> 
>  docs/storage.html.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Automatic probing of volumes is/was a security issue and removed during
v4.5.0 - the last vestiges were commit e1e8d0a9b with the qemu.conf
adjustment in commit c95f50cb.

In any case, with respect to the patch in particular...

"dos" is the expected <format type='%s'...> value; however, "msdos" is
the value used by "parted mklabel --script msdos" (see
virStorageBackendDiskBuildPool)

So that line in the storage page for the Disk pool description isn't
entirely wrong, but it is somewhat misleading. I would say it's more
that the pool format is dos which generates an on disk partition format
of msdos (or some sort of wordsmithing).

In any case, I'm OK with changing to just "dos", but I won't complain if
the partition format verbiage is added.

Reviewed-by: John Ferlan <jferlan@redhat.com>
SFF,

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Fix mention of disk pool default
Posted by Eric Blake 5 years, 7 months ago
On 08/30/2018 08:41 AM, John Ferlan wrote:

> 
> "dos" is the expected <format type='%s'...> value; however, "msdos" is
> the value used by "parted mklabel --script msdos" (see
> virStorageBackendDiskBuildPool)
> 
> So that line in the storage page for the Disk pool description isn't
> entirely wrong, but it is somewhat misleading. I would say it's more
> that the pool format is dos which generates an on disk partition format
> of msdos (or some sort of wordsmithing).

How about squashing this in?  That way, you can still grep for 'msdos'.

diff --git i/docs/storage.html.in w/docs/storage.html.in
index 6ec623831c..9adcc2a87f 100644
--- i/docs/storage.html.in
+++ w/docs/storage.html.in
@@ -357,8 +357,10 @@
        </li>
      </ul>
      <p>
-      The <code>dos</code> or <code>gpt</code> formats are recommended for
-      best portability - the latter is needed for disks larger than 2TB.
+      The formats <code>dos</code> ("msdos" in parted terminology,
+      good for BIOS systems) or <code>gpt</code> (good for UEFI
+      systems) are recommended for best portability - the latter is
+      needed for disks larger than 2TB.
      </p>

      <h3>Valid volume format types</h3>


> 
> In any case, I'm OK with changing to just "dos", but I won't complain if
> the partition format verbiage is added.
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> SFF,
> 
> John
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Fix mention of disk pool default
Posted by John Ferlan 5 years, 7 months ago

On 08/30/2018 10:04 AM, Eric Blake wrote:
> On 08/30/2018 08:41 AM, John Ferlan wrote:
> 
>>
>> "dos" is the expected <format type='%s'...> value; however, "msdos" is
>> the value used by "parted mklabel --script msdos" (see
>> virStorageBackendDiskBuildPool)
>>
>> So that line in the storage page for the Disk pool description isn't
>> entirely wrong, but it is somewhat misleading. I would say it's more
>> that the pool format is dos which generates an on disk partition format
>> of msdos (or some sort of wordsmithing).
> 
> How about squashing this in?  That way, you can still grep for 'msdos'.
> 
> diff --git i/docs/storage.html.in w/docs/storage.html.in
> index 6ec623831c..9adcc2a87f 100644
> --- i/docs/storage.html.in
> +++ w/docs/storage.html.in
> @@ -357,8 +357,10 @@
>        </li>
>      </ul>
>      <p>
> -      The <code>dos</code> or <code>gpt</code> formats are recommended for
> -      best portability - the latter is needed for disks larger than 2TB.
> +      The formats <code>dos</code> ("msdos" in parted terminology,
> +      good for BIOS systems) or <code>gpt</code> (good for UEFI
> +      systems) are recommended for best portability - the latter is
> +      needed for disks larger than 2TB.
>      </p>
> 
>      <h3>Valid volume format types</h3>
> 
> 

Looks good to me.

John

>>
>> In any case, I'm OK with changing to just "dos", but I won't complain if
>> the partition format verbiage is added.
>>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>> SFF,
>>
>> John
>>
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list