The patch description is lacking some explanation or reasoning about why
the logical IDs are being added to the query. It would be particularly
valuable to have an example of which use cases they are meant to serve.

This leads to my primary concern: that the logical IDs are an internal data
structure, and as soon as we make it a part of the interface, we enshrine
the format and cannot change it at our leisure. While we do leak the IDs
through the examination of jobs results, no one would think this to be a
proper part of the interface.

What is this extension needed for?

On Fri, Jan 15, 2016 at 4:02 PM, 'Lisa Velden' via ganeti-devel <
[email protected]> wrote:

> Add the logical ids of an instance's disks to the RAPI output of basic
> instance information.
>
> Signed-off-by: Lisa Velden <[email protected]>
> ---
>  lib/query.py                 |  3 +++
>  lib/rapi/rlib2.py            |  2 +-
>  src/Ganeti/Query/Instance.hs | 11 +++++++++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/lib/query.py b/lib/query.py
> index 085a60d..c0dea20 100644
> --- a/lib/query.py
> +++ b/lib/query.py
> @@ -2008,6 +2008,9 @@ def _GetInstanceDiskFields():
>       IQ_CONFIG, 0, lambda ctx, inst: [disk.name for disk in inst.disks]),
>      (_MakeField("disk.uuids", "Disk_UUIDs", QFT_OTHER, "List of disk
> UUIDs"),
>       IQ_CONFIG, 0, lambda ctx, inst: [disk.uuid for disk in inst.disks]),
> +    (_MakeField("disk.logical_ids", "Disk_logical_ids", QFT_OTHER,
> +                "List of disk logical ids"),
> +     IQ_CONFIG, 0, lambda ctx, inst: [disk.logical_id for disk in
> inst.disks]),
>      ]
>
>    # Disks by number
> diff --git a/lib/rapi/rlib2.py b/lib/rapi/rlib2.py
> index fb1ec14..339e63b 100644
> --- a/lib/rapi/rlib2.py
> +++ b/lib/rapi/rlib2.py
> @@ -83,7 +83,7 @@ I_FIELDS = ["name", "admin_state", "os",
>              "nic.links", "nic.networks", "nic.networks.names",
> "nic.bridges",
>              "network_port",
>              "disk.sizes", "disk.spindles", "disk_usage", "disk.uuids",
> -            "disk.names",
> +            "disk.names", "disk.logical_ids",
>              "beparams", "hvparams",
>              "oper_state", "oper_ram", "oper_vcpus", "status",
>              "custom_hvparams", "custom_beparams", "custom_nicparams",
> diff --git a/src/Ganeti/Query/Instance.hs b/src/Ganeti/Query/Instance.hs
> index 4d2e660..8c1b72e 100644
> --- a/src/Ganeti/Query/Instance.hs
> +++ b/src/Ganeti/Query/Instance.hs
> @@ -207,6 +207,9 @@ instanceFields =
>    , (FieldDefinition "disk.uuids" "Disk_UUIDs" QFTOther
>       "List of disk UUIDs",
>       FieldConfig getDiskUuids, QffNormal)
> +  , (FieldDefinition "disk.logical_ids" "Disk_logical_ids" QFTOther
> +     "List of disk logical ids",
> +     FieldConfig getDiskLogicalIds, QffNormal)
>      -- For pre-2.14 backwards compatibility
>    , (FieldDefinition "disk_template" "Disk_template" QFTText
>       "Instance disk template",
> @@ -416,6 +419,14 @@ getDiskUuids :: ConfigData -> Instance -> ResultEntry
>  getDiskUuids cfg =
>    rsErrorNoData . liftA (map uuidOf) . getInstDisksFromObj cfg
>
> +-- | Get a list of disk logical IDs for an instance
> +getDiskLogicalIds :: ConfigData -> Instance -> ResultEntry
> +getDiskLogicalIds cfg =
> +  rsErrorNoData . liftA (map $ MaybeForJSON .
> +                         encodedDLId) . getInstDisksFromObj cfg
> + where
> +  encodedDLId x = encodeDLId <$> diskLogicalId x
> +
>  -- | Creates a functions which produces a FieldConfig 'FieldGetter' when
> fed
>  -- an index. Works for fields that may not return a value, expressed
> through
>  -- the Maybe monad.
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

Reply via email to