On 10/15/2012 6:58 PM, Constantinos Venetsanopoulos wrote:
Hi iustin,
I just sent you the interdiff. After rebasing, there is a trivial
conflict
when applying the next commit (2/7). I can resend it too, if you want.
I meant '3/7' here: Multiple ExtStorage providers
cven
Finally, I'll also update the 'shared-filer' ExtStorage provider I
sent you,
to include a setinfo script.
Regards,
Constantinos
On 10/10/2012 12:15 PM, Iustin Pop wrote:
On Wed, Oct 10, 2012 at 12:07:31PM +0300, Constantinos
Venetsanopoulos wrote:
On 10/09/2012 07:10 PM, Iustin Pop wrote:
On Tue, Oct 09, 2012 at 06:59:55PM +0300, Constantinos
Venetsanopoulos wrote:
On 10/08/2012 04:55 PM, Iustin Pop wrote:
On Thu, Sep 27, 2012 at 05:43:56PM +0300, Constantinos
Venetsanopoulos wrote:
On 09/27/2012 04:17 PM, Iustin Pop wrote:
On Wed, Sep 26, 2012 at 05:38:18PM +0300, Constantinos
Venetsanopoulos wrote:
With this commit we introduce the External Storage Interface
to Ganeti, abbreviated: ExtStorage Interface.
The ExtStorage Interface provides Ganeti with the ability to
interact
with externally connected shared storage pools, visible by all
VM-capable nodes. This means that Ganeti is able to handle VM
disks
that reside inside a NAS/SAN or any distributed block storage
provider.
The ExtStorage Interface provides a clear API, heavily
inspired by the
gnt-os-interface API, that can be used by storage vendors or
sysadmins
to write simple ExtStorage Providers (correlated to
gnt-os-interface's
OS Definitions). Those Providers will glue externally attached
shared
storage with Ganeti, without the need of preprovisioned block
devices
on Ganeti VM-capable nodes as confined be the current
`blockdev' disk
template.
To do so, we implement a new disk template called `ext' (of type
DTS_EXT_MIRROR) that passes control to externally provided
scripts
(the ExtStorage Provider) for the template's basic functions:
create / attach / detach / remove / grow
The scripts reside under ES_SEARCH_PATH (correlated to
OS_SEARCH_PATH)
and only one ExtStorage Provider is supported called `ext'.
The disk's logical id is the tuple ('ext', UUID.ext.diskX),
where UUID
is generated as in disk template `plain' and X is the disk's
index.
A few general comments, or rather questions. Some of the are
rather
design level, but they become more apparent when reading the
code only,
sorry.
No problem.
I'm not sure creating one log file per operation is a good
idea. While
installing/renaming an instance is a (somewhat) rare operation,
activating the disks of an instance is a much often one. I know
that
right now in attach you don't create a log, but that seems to
be the
intention (hence the TODO). Thoughts?
Indeed, creating a log file during attach was the initial
intention, however
I understand your point since specifically attach is run
multiple times
even during a single instance addition. On the other hand, I
think creation
and removal of a disk should reside in different files because
they do not
happen so often. Furthermore, these log files are the only place
where the
admin actually can see what happens, when something goes wrong with
the external hardware and are also very helpful when writing custom
ExtStorage providers.
I'm not sure I understand this. Do you mean about
debug/informational
messages? I believe that actual errors will simply propagate to
ganeti
as failures in activation/other ops.
Actual errors do propagate to ganeti. However the log doesn't
(except the "n" last lines of the logfile). The user has to go
back to
the log file to see what happened exactly. From my experience writing
ExtStorage providers, it really helps being able to look at the
attach
script's stderr.
I see.
What do you think? Another option would be to keep the log files
for
create/remove/grow and do not log the attach/detach. Or keep them
for now to see the workflow and if they produce a lot of
unnecessary
info that we do not want to keep, merge all actions in a single
log file
for each disk (but this needs code refactoring).
Indeed.
Maybe a separate option would be, as you say, log only
create/remove/grow and recommend that attach/detach options log to
syslog? Or request that all logging goes to syslog?
After more consideration, I think that not logging attach/detach
ops is
not the right thing to do.
Currently no logging for attach happens, due to the issue with
RunResult().
When this gets fixed, I think it would be best to log everything
related
to a single disk in the same log file, so we don't miss anything,
keep
related things together, and do not create too many small files.
As an
example the file would contain a series of
[create,attach,attach,attach,detach,attach,...,grow,...,detach,remove]
ops.
If you are OK with this approach, I will contribute the relevant
patches as
soon as the code gets merged and the issue in the TODO comment
gets fixed.
Sounds good. Thanks for taking the time to think this through!
Also, it seems by reading the code that Ganeti generates an
UUID for the
volume, and it asks the provider to create one such drive;
basically,
the provider will get "please create disk f0559a.0 of size
500G", right?
That's right.
This means that only the ganeti configuration has the mapping
instance
name to disks, and that losing the ganeti configuration would mean
instantly losing all instance data since we can't map it back
to a VM.
For 'plain' disks, we explicitly tag the LV names with the name
of the
instance, to safeguard against such problems. Do you think this
is not a
problem?
To be honest, I haven't thought about this case, but I understand
your point.
Or could we add the instance name to the external provider
create call, such that providers could do such an association
themselves, if they care?
That would be fine, but I can't see an easy way to do that,
since the
instance object doesn't find its way inside bdev, and it
wouldn't be clean
to add new parameters in the Create call. What we could do though,
would be to prepend the volume name with the instance's name.
Much like you do with 'plain' disks. Now the disk name has the
following
format:
'04859fac4.ext.diskX' where X is the index.
We could change that to 'instancename-04859fac4.ext.diskX' and
document it appropriately, so that the mapping will be there for
the
providers to use it, if they care. Would that be OK?
Hmm, not really. The instance name can and will change, so having
it as
part of the disk ID is not good.
But note that the instance "info" is already passed to
BlockDevCreate,
and while not passed to the actual Create call, is passed to
BlockDevice.SetInfo()? Could we reuse that here too?
If I understand correctly, you prefer disk names to be independent of
the instance name, then go and tag the disk object itself, e.g.,
the lv in
the case of lvm.
Yes.
A similar approach would be for me to implement the
ExtStorageDevice.SetInfo() call which runs an external "setinfo"
script.
The setinfo script would mark the disk with the instance name or
other
metadata in a provider-specific way. For example the volumes of a NAS
would be marked with vendor specific metadata and the admin would be
able to recover instance data by examining these.
Yes, exactly.
However, even if this happens, it seems I can only find code
setting lv
tags during blockdev creation and not instance rename. So, how do
lv tags stay consistent after renames? Thus, even we follow the
approach
you propose, the admin will still have problems tracking down
instance
disks after loosing the ganeti config. Thoughts?
That is actually a current bug (only internally raised so far). We do
plan to fix the rename LU to include this, by exposing SetInfo via an
rpc call.
So yes, let's take this approach, and fix the rename issue separately.
OK. So, to sum up, I'll implement the SetInfo functionality, fix all
the
small issues we discussed in previous emails and send an interdiff.
When we fix the RunResult TODO and the code gets merged, I'll send
a patch that will merge all log files into a single one for each disk.
Finally, after the merge, I will also send a separate patch that
documents the SetInfo functionality in the design doc and man pages.
Is that OK?
Sounds good, thanks!
iustin
