Ryan O''Connor
2009-Jul-03 06:35 UTC
[Xen-devel] [PATCH] blktap2: fix save/restore/migration
# HG changeset patch # User Ryan O''Connor <rjo@cs.ubc.ca> # Date 1246602456 25200 # Node ID 2292564d6029d5a3fc6b57d51059e2bd5e0dfbec # Parent 6b489d139da0dd3bd4a1f673816559acf9b0a9e0 blktap2: fix save/restore/migration blktap2 devices use a regular ''phy'' vbd blkback backend, causing Blktap2Controller to trample the devices'' parameters. This causes problems with save/restore and managed domains, among other things. This patch modifies Blktap2Controller to store both the vbd and tap2 parameters in xenstore, and stops it from trampling the device''s config on device creation. * store blktap2 parameters in xenstore * restore blktap2 device config to original state once the underlying vbd device is created (this fixes managed domains) * use blktap2 parameters rather than vbd parameters when building blktap2 device configurations * remove blktap2 specific code from XendConfig Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca> diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/XendConfig.py --- a/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:33 2009 -0700 +++ b/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:36 2009 -0700 @@ -1120,8 +1120,6 @@ class XendConfig(dict): if sxp.child_value(config, ''bootable'', None) is None: is_bootable = dev_cfg.get(''bootable'', 0) config.append([''bootable'', int(is_bootable)]) - if dev_cfg.has_key(''required_uname''): - config.append([''required_uname'', dev_cfg[''required_uname'']]) config.append([''VDI'', dev_cfg.get(''VDI'', '''')]) sxpr.append([''device'', config]) @@ -1372,13 +1370,6 @@ class XendConfig(dict): dev_info[''driver''] = ''paravirtualised'' if dev_type == ''tap'' or dev_type == ''tap2'': - if dev_info.has_key(''required_uname''): - # Restore uname by required_uname because uname might - # be replaced with ''phy:/dev/xen/blktap-2/tapdev*''. - dev_info[''uname''] = dev_info[''required_uname''] - else: - # Save uname for next domain start. - dev_info[''required_uname''] = dev_info[''uname''] tap_disk_type = dev_info[''uname''].split('':'')[1] # tapdisk uname may be ''tap:<driver>'' or ''tap:tapdisk:<driver>'' if tap_disk_type == ''tapdisk'': diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/server/BlktapController.py --- a/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:33 2009 -0700 +++ b/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:36 2009 -0700 @@ -137,10 +137,39 @@ class Blktap2Controller(BlktapController deviceClass, self.vm.getDomid(), devid) + def getDeviceDetails(self, config): + + (devid, back, front) = BlktapController.getDeviceDetails(self, config) + if self.deviceClass == ''tap2'': + # since blktap2 uses blkback as a backend the ''params'' feild contains + # the path to the blktap2 device (/dev/xen/blktap-2/tapdev*). As well, + # we need to store the params used to create the blktap2 device + # (tap:tapdisk:<driver>:/<image-path>) + tapdisk_uname = config.get(''tapdisk_uname'', '''') + (_, tapdisk_params) = string.split(tapdisk_uname, '':'', 1) + back[''tapdisk-params''] = tapdisk_params + + return (devid, back, front) + + def getDeviceConfiguration(self, devid, transaction = None): + + # this is a blktap2 device, so we need to overwrite the ''params'' feild + # with the actual blktap2 parameters. (the vbd parameters are of little + # use to us) + config = BlktapController.getDeviceConfiguration(self, devid, transaction) + if transaction is None: + tapdisk_params = self.readBackend(devid, ''tapdisk-params'') + else: + tapdisk_params = self.readBackendTxn(transaction, devid, ''tapdisk-params'') + if tapdisk_params: + config[''uname''] = ''tap:'' + tapdisk_params + + return config + def createDevice(self, config): - uname = config.get(''required_uname'', '''') + uname = config.get(''uname'', '''') try: (typ, subtyp, params, file) = string.split(uname, '':'', 3) except: @@ -180,11 +209,15 @@ class Blktap2Controller(BlktapController stdout.close(); stderr.close(); - #modify the configuration to attach as a vbd, now that the - #device is configured. Then continue to create the device + # modify the configutration to create a blkback for the underlying + # blktap2 device. Note: we need to preserve the original tapdisk uname + # (it is used during save/restore and for managed domains). + config.update({''tapdisk_uname'' : uname}) config.update({''uname'' : ''phy:'' + device.rstrip()}) devid = BlkifController.createDevice(self, config) + config.update({''uname'' : uname}) + config.pop(''tapdisk_uname'') return devid # The new blocktap implementation requires a sysfs signal to close _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ryan O''Connor
2009-Jul-03 06:40 UTC
Re: [Xen-devel] [PATCH] blktap2: fix save/restore/migration
My apologies to all, This patch depends on my previous patch (seperate blktap1/blktap2 disk types). I should have sent them out together. Thanks, Ryan On Thu, Jul 02, 2009 at 11:35:47PM -0700, Ryan O''Connor wrote:> # HG changeset patch > # User Ryan O''Connor <rjo@cs.ubc.ca> > # Date 1246602456 25200 > # Node ID 2292564d6029d5a3fc6b57d51059e2bd5e0dfbec > # Parent 6b489d139da0dd3bd4a1f673816559acf9b0a9e0 > blktap2: fix save/restore/migration > > blktap2 devices use a regular ''phy'' vbd blkback backend, causing > Blktap2Controller to trample the devices'' parameters. This causes problems with > save/restore and managed domains, among other things. This patch modifies > Blktap2Controller to store both the vbd and tap2 parameters in xenstore, and > stops it from trampling the device''s config on device creation. > > * store blktap2 parameters in xenstore > * restore blktap2 device config to original state once the underlying vbd > device is created (this fixes managed domains) > * use blktap2 parameters rather than vbd parameters when building blktap2 > device configurations > * remove blktap2 specific code from XendConfig > > Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca> > > diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/XendConfig.py > --- a/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:33 2009 -0700 > +++ b/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:36 2009 -0700 > @@ -1120,8 +1120,6 @@ class XendConfig(dict): > if sxp.child_value(config, ''bootable'', None) is None: > is_bootable = dev_cfg.get(''bootable'', 0) > config.append([''bootable'', int(is_bootable)]) > - if dev_cfg.has_key(''required_uname''): > - config.append([''required_uname'', dev_cfg[''required_uname'']]) > config.append([''VDI'', dev_cfg.get(''VDI'', '''')]) > > sxpr.append([''device'', config]) > @@ -1372,13 +1370,6 @@ class XendConfig(dict): > dev_info[''driver''] = ''paravirtualised'' > > if dev_type == ''tap'' or dev_type == ''tap2'': > - if dev_info.has_key(''required_uname''): > - # Restore uname by required_uname because uname might > - # be replaced with ''phy:/dev/xen/blktap-2/tapdev*''. > - dev_info[''uname''] = dev_info[''required_uname''] > - else: > - # Save uname for next domain start. > - dev_info[''required_uname''] = dev_info[''uname''] > tap_disk_type = dev_info[''uname''].split('':'')[1] > # tapdisk uname may be ''tap:<driver>'' or ''tap:tapdisk:<driver>'' > if tap_disk_type == ''tapdisk'': > diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/server/BlktapController.py > --- a/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:33 2009 -0700 > +++ b/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:36 2009 -0700 > @@ -137,10 +137,39 @@ class Blktap2Controller(BlktapController > deviceClass, > self.vm.getDomid(), devid) > > + def getDeviceDetails(self, config): > + > + (devid, back, front) = BlktapController.getDeviceDetails(self, config) > + if self.deviceClass == ''tap2'': > + # since blktap2 uses blkback as a backend the ''params'' feild contains > + # the path to the blktap2 device (/dev/xen/blktap-2/tapdev*). As well, > + # we need to store the params used to create the blktap2 device > + # (tap:tapdisk:<driver>:/<image-path>) > + tapdisk_uname = config.get(''tapdisk_uname'', '''') > + (_, tapdisk_params) = string.split(tapdisk_uname, '':'', 1) > + back[''tapdisk-params''] = tapdisk_params > + > + return (devid, back, front) > + > + def getDeviceConfiguration(self, devid, transaction = None): > + > + # this is a blktap2 device, so we need to overwrite the ''params'' feild > + # with the actual blktap2 parameters. (the vbd parameters are of little > + # use to us) > + config = BlktapController.getDeviceConfiguration(self, devid, transaction) > + if transaction is None: > + tapdisk_params = self.readBackend(devid, ''tapdisk-params'') > + else: > + tapdisk_params = self.readBackendTxn(transaction, devid, ''tapdisk-params'') > + if tapdisk_params: > + config[''uname''] = ''tap:'' + tapdisk_params > + > + return config > + > > def createDevice(self, config): > > - uname = config.get(''required_uname'', '''') > + uname = config.get(''uname'', '''') > try: > (typ, subtyp, params, file) = string.split(uname, '':'', 3) > except: > @@ -180,11 +209,15 @@ class Blktap2Controller(BlktapController > stdout.close(); > stderr.close(); > > - #modify the configuration to attach as a vbd, now that the > - #device is configured. Then continue to create the device > + # modify the configutration to create a blkback for the underlying > + # blktap2 device. Note: we need to preserve the original tapdisk uname > + # (it is used during save/restore and for managed domains). > + config.update({''tapdisk_uname'' : uname}) > config.update({''uname'' : ''phy:'' + device.rstrip()}) > > devid = BlkifController.createDevice(self, config) > + config.update({''uname'' : uname}) > + config.pop(''tapdisk_uname'') > return devid > > # The new blocktap implementation requires a sysfs signal to close > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel-- Ryan O''Connor <rjo@cs.ubc.ca> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Masaki Kanno
2009-Jul-03 08:21 UTC
Re: [Xen-devel] [PATCH] blktap2: fix save/restore/migration
Great! I''m impressed by your patches!! Best regards, Kan Thu, 02 Jul 2009 23:35:47 -0700, "Ryan O''Connor" wrote:># HG changeset patch ># User Ryan O''Connor <rjo@cs.ubc.ca> ># Date 1246602456 25200 ># Node ID 2292564d6029d5a3fc6b57d51059e2bd5e0dfbec ># Parent 6b489d139da0dd3bd4a1f673816559acf9b0a9e0 >blktap2: fix save/restore/migration > >blktap2 devices use a regular ''phy'' vbd blkback backend, causing >Blktap2Controller to trample the devices'' parameters. This causes problems >with >save/restore and managed domains, among other things. This patch modifies >Blktap2Controller to store both the vbd and tap2 parameters in xenstore, and >stops it from trampling the device''s config on device creation. > > * store blktap2 parameters in xenstore > * restore blktap2 device config to original state once the underlying vbd > device is created (this fixes managed domains) > * use blktap2 parameters rather than vbd parameters when building blktap2 > device configurations > * remove blktap2 specific code from XendConfig > >Signed-off-by: Ryan O''Connor <rjo@cs.ubc.ca> > >diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/XendConfig.py >--- a/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:33 2009 -0700 >+++ b/tools/python/xen/xend/XendConfig.py Thu Jul 02 23:27:36 2009 -0700 >@@ -1120,8 +1120,6 @@ class XendConfig(dict): > if sxp.child_value(config, ''bootable'', > None) is None: > is_bootable = dev_cfg.get( >''bootable'', 0) > config.append([''bootable'', int( >is_bootable)]) >- if dev_cfg.has_key(''required_uname''): >- config.append([''required_uname'', >dev_cfg[''required_uname'']]) > config.append([''VDI'', dev_cfg.get( >''VDI'', '''')]) > > sxpr.append([''device'', config]) >@@ -1372,13 +1370,6 @@ class XendConfig(dict): > dev_info[''driver''] = ''paravirtualised'' > > if dev_type == ''tap'' or dev_type == ''tap2'': >- if dev_info.has_key(''required_uname''): >- # Restore uname by required_uname because uname might >- # be replaced with ''phy:/dev/xen/blktap-2/tapdev*''. >- dev_info[''uname''] = dev_info[''required_uname''] >- else: >- # Save uname for next domain start. >- dev_info[''required_uname''] = dev_info[''uname''] > tap_disk_type = dev_info[''uname''].split('':'')[1] > # tapdisk uname may be ''tap:<driver>'' or ''tap:tapdisk:< >driver>'' > if tap_disk_type == ''tapdisk'': >diff -r 6b489d139da0 -r 2292564d6029 tools/python/xen/xend/server/ >BlktapController.py >--- a/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:33 >2009 -0700 >+++ b/tools/python/xen/xend/server/BlktapController.py Thu Jul 02 23:27:36 >2009 -0700 >@@ -137,10 +137,39 @@ class Blktap2Controller(BlktapController > deviceClass, > self.vm.getDomid(), devid) > >+ def getDeviceDetails(self, config): >+ >+ (devid, back, front) = BlktapController.getDeviceDetails(self, >config) >+ if self.deviceClass == ''tap2'': >+ # since blktap2 uses blkback as a backend the ''params'' feild >contains >+ # the path to the blktap2 device (/dev/xen/blktap-2/tapdev*). As >well, >+ # we need to store the params used to create the blktap2 device >+ # (tap:tapdisk:<driver>:/<image-path>) >+ tapdisk_uname = config.get(''tapdisk_uname'', '''') >+ (_, tapdisk_params) = string.split(tapdisk_uname, '':'', 1) >+ back[''tapdisk-params''] = tapdisk_params >+ >+ return (devid, back, front) >+ >+ def getDeviceConfiguration(self, devid, transaction = None): >+ >+ # this is a blktap2 device, so we need to overwrite the ''params'' >feild >+ # with the actual blktap2 parameters. (the vbd parameters are of >little >+ # use to us) >+ config = BlktapController.getDeviceConfiguration(self, devid, >transaction) >+ if transaction is None: >+ tapdisk_params = self.readBackend(devid, ''tapdisk-params'') >+ else: >+ tapdisk_params = self.readBackendTxn(transaction, devid, >''tapdisk-params'') >+ if tapdisk_params: >+ config[''uname''] = ''tap:'' + tapdisk_params >+ >+ return config >+ > > def createDevice(self, config): > >- uname = config.get(''required_uname'', '''') >+ uname = config.get(''uname'', '''') > try: > (typ, subtyp, params, file) = string.split(uname, '':'', 3) > except: >@@ -180,11 +209,15 @@ class Blktap2Controller(BlktapController > stdout.close(); > stderr.close(); > >- #modify the configuration to attach as a vbd, now that the >- #device is configured. Then continue to create the device >+ # modify the configutration to create a blkback for the underlying >+ # blktap2 device. Note: we need to preserve the original tapdisk >uname >+ # (it is used during save/restore and for managed domains). >+ config.update({''tapdisk_uname'' : uname}) > config.update({''uname'' : ''phy:'' + device.rstrip()}) > > devid = BlkifController.createDevice(self, config) >+ config.update({''uname'' : uname}) >+ config.pop(''tapdisk_uname'') > return devid > > # The new blocktap implementation requires a sysfs signal to close > >_______________________________________________ >Xen-devel mailing list >Xen-devel@lists.xensource.com >http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel