[pve-devel] Implement "resize" for the DRBD backend.

[Philipp Marek]

> > Please see attached a patch to implement "resize" for the DRBD backend.
> > 
> > I hope it matches all your coding style guidelines;
> > feedback is welcome, of course.
> 
> Please can you send patches inline? That way it is easier to review code
> and add comments. I copied the code for this purpose - comments inline:
Okay, will do so.

> diff --git a/PVE/Storage/DRBDPlugin.pm b/PVE/Storage/DRBDPlugin.pm
> index 91420da..d031db6 100644
> --- a/PVE/Storage/DRBDPlugin.pm
> +++ b/PVE/Storage/DRBDPlugin.pm
> @@ -356,19 +356,19 @@ sub deactivate_volume {
>  }
>  
>  sub volume_resize {
> -    my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
> +    my ($class, $scfg, $storeid, $name, $size, $running) = @_;
> 
> why do you rename $volname to $name?
>  
> -    $size = ($size/1024/1024) . "M";
> +    $size = int($size/1024);
> +    my $hdl = connect_drbdmanage_service();
>  
> -    my $path = $class->path($scfg, $volname);
> +    die "illegal name '$name' - should be 'vm-*-*'\n"
> +	if $name !~ m/^vm-\d+-/;
> 
> And reason for above check? Or did you just copied that code from alloc_image?
Right. And that's why I changed the name - to keep it the same.
Is that a bad approach?

> -    # fixme: howto implement this
> -    die "drbd volume_resize is not implemented";
> -    
> -    #my $cmd = ['/sbin/lvextend', '-L', $size, $path];
> -    #run_command($cmd, errmsg => "error resizing volume '$path'");
> +    # FIXME if there's ever more than one volume in a resource
> 
> not sure if we ever want to support multiple volumes inside one resource?
> Why would we want to do that?
So that the volumes use a common write stream across the network.

If you have eg. a database that uses 3 volumes (data, log data, write-ahead 
log), you want to have these three at the *same* point in time.

When one of 3 connections breaks, then the other two volumes could run 
ahead - and if the Primary node breaks down at that time, the Secondary 
wouldn't have 3 consistent volumes, which might lead to troubles.


> Anyways, I tested the patch, and volume_resize() now returns without errors.
> The problem is that it does not resize the underlying LVM volume.
> 
> Do I need to install any drbdmanage updates/patches to make that work?
Well, as the commit message says, there's a small problem in 
DRBD/drbdmanage right now.
drbdmanage uses the "size" parameter in the generated config files, to 
ensure an *exact* net device size; sadly this triggers a bug in DRBD, and 
so the resizing doesn't work.
Because of this the drbdmanage code for resizing isn't in the master branch 
(yet); as soon as DRBD 9 works with that parameter too, we'll push this 
code.


I'm aware that this makes my patch not overly useful; I wanted to have 
a small first patch, so that I can see the coding conventions.


Regards,

Phil